Skip to content

Commit

Permalink
fix(Message::appendText): QRegularExpression whitespace bug
Browse files Browse the repository at this point in the history
Unlike QRegExp, when a QRegularExpression starts with \s,
it may match whitespace *including newlines*.

So when looking for the start of a section, we might match
"\n\n == January 2025 ==" instead of " == January 2025 =="

Then the logic that tries to skip past the header, would instead
skip past the first \n, which is still _before_ the header.
Then we would think the target header is the start of the next section,
and we would end up inserting a message just before the section
it was intended to be appended to.

QRegularExpression is available since Qt 5.0 and is recommended.
Let's get rid of the old QRegExp and fix the QRegularExpression usage.
  • Loading branch information
tux3 committed Jan 17, 2025
1 parent cee5d7a commit a4430e7
Showing 1 changed file with 14 additions and 29 deletions.
43 changes: 14 additions & 29 deletions src/huggle_core/message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "wikisite.hpp"
#include "wikiutil.hpp"
#include "wikiuser.hpp"
#include <QRegularExpression>
#include <QUrl>

using namespace Huggle;
Expand Down Expand Up @@ -205,7 +206,7 @@ void Message::processNextStep()
this->Error = MessageError_ArticleExist;
} else
{
this->Fail(_l("error-unknown-code",ec));
this->Fail(_l("error-unknown-code", ec));
this->Error = MessageError_Unknown;
}
return;
Expand Down Expand Up @@ -319,17 +320,15 @@ void Message::processSend()
if (this->SectionKeep)
{
this->Text = this->appendText(this->Text, this->originalUnmodifiedPageText, this->Title);
} else
{
} else {
// original page needs to be included in new value
if (!this->originalUnmodifiedPageText.isEmpty())
this->Text = this->originalUnmodifiedPageText + "\n\n" + this->Text;
}
this->query->Parameters = "title=" + QUrl::toPercentEncoding(User->GetTalk()) + "&summary=" + QUrl::toPercentEncoding(summary)
+ "&text=" + QUrl::toPercentEncoding(this->Text) + parameters
+ "&token=" + QUrl::toPercentEncoding(this->User->GetSite()->GetProjectConfig()->Token_Csrf);
}else
{
} else {
this->query->Parameters = "title=" + QUrl::toPercentEncoding(User->GetTalk()) + "&section=new&sectiontitle="
+ QUrl::toPercentEncoding(this->Title) + "&summary=" + QUrl::toPercentEncoding(summary)
+ "&text=" + QUrl::toPercentEncoding(this->Text) + parameters + "&token="
Expand Down Expand Up @@ -357,8 +356,7 @@ void Message::processTalkPageRetrieval()
this->originalUnmodifiedPageText = page->Value;
this->previousTalkPageRetrieved = true;
return;
} else
{
} else {
if (!missing)
{
Huggle::Syslog::HuggleLogs->DebugLog(this->query->Result->Data);
Expand All @@ -375,12 +373,10 @@ QString Message::appendText(QString text, QString original_text, QString section
// there is nothing to insert this to
return original_text += "\n\n" + text + "\n\n";
}
#ifdef QT6_BUILD

QRegularExpression regex("\\s*==\\s*" + QRegularExpression::escape(section_name) + "\\s*==");
#else
QRegExp regex("\\s*==\\s*" + QRegExp::escape(section_name) + "\\s*==");
#endif
if (!original_text.contains(regex))
QRegularExpressionMatch regex_match;
if (!original_text.contains(regex, &regex_match))
{
// there is no section to append to
if (!original_text.isEmpty())
Expand All @@ -390,32 +386,21 @@ QString Message::appendText(QString text, QString original_text, QString section
original_text += "== " + section_name + " ==\n\n" + text;
return original_text;
}
#ifdef QT6_BUILD
QRegularExpression header("\\s*==.*==\\s*");
#else
QRegExp header("\\s*==.*==\\s*");
#endif
int start_of_desired_section = original_text.lastIndexOf(regex);

// we need to check if there is any other section after this one
int start_of_desired_section = original_text.lastIndexOf(regex) + regex_match.capturedView().length();
QString section = original_text.mid(start_of_desired_section);
if (section.contains("\n"))
{
// cut the header text
int Diff = section.indexOf("\n") + 1;
start_of_desired_section += Diff;
section = section.mid(Diff);
}
QRegularExpression next_header("\\s*==.*==\\s*");
int start_point = start_of_desired_section;
if (section.contains(header))
if (section.contains(next_header))
{
// yes there is some other section, so we need to know where it is, so that we know where current
// section ends (we want to append text to bottom of current section)
start_point += section.indexOf(header);
start_point += section.indexOf(next_header);
} else
{
start_point += section.length();
}
// write the text exactly after the start point, but leave some space after it
original_text = original_text.insert(start_point, "\n\n" + text + "\n");
return original_text;
return original_text.insert(start_point, "\n\n" + text + "\n");
}

0 comments on commit a4430e7

Please sign in to comment.