Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

singleurl: fix query_is_modified #323

Merged
merged 1 commit into from
Aug 16, 2024
Merged

singleurl: fix query_is_modified #323

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 34 additions & 30 deletions trurl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,6 @@ static void trurl_warnf(struct option *o, const char *fmt, ...)
struct string qpairs[MAX_QPAIRS]; /* encoded */
struct string qpairsdec[MAX_QPAIRS]; /* decoded */
int nqpairs; /* how many is stored */
bool query_is_modified = false;

static void trurl_cleanup_options(struct option *o)
{
Expand Down Expand Up @@ -1134,8 +1133,9 @@ static void json(struct option *o, CURLU *uh)
}

/* --trim query="utm_*" */
static void trim(struct option *o)
static bool trim(struct option *o)
{
bool query_is_modified = false;
struct curl_slist *node;
for(node = o->trim_list; node; node = node->next) {
char *ptr;
Expand Down Expand Up @@ -1163,7 +1163,7 @@ static void trim(struct option *o)
removed. Get a copy and edit that accordingly. */
temp = strdup(ptr);
if(!temp)
return; /* out of memory, bail out */
return query_is_modified; /* out of memory, bail out */
Copy link
Collaborator Author

@emanuele6 emanuele6 Aug 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't continue make more sense if it does not just fail? Or wouldn't it make more sense to just error instead of possibly outputting a wrong result?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either this should exit immediately with an error, or we need to make the function return an error code so that the parent caller can spot the error. We should not let it continue and output wrong.

Copy link
Collaborator Author

@emanuele6 emanuele6 Aug 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently this return is just ignored, as if this and the following --trims didn't match anything

temp[inslen - 2] = '*';
temp[inslen - 1] = '\0';
ptr = temp;
Expand Down Expand Up @@ -1192,12 +1192,13 @@ static void trim(struct option *o)
qpairs[i].len = 0;
qpairsdec[i].str = strdup(""); /* marked as deleted */
qpairsdec[i].len = 0;
query_is_modified = true;
}
}
free(temp);
}
query_is_modified = true;
}
return query_is_modified;
}

/* memdup the amount and add a trailing zero */
Expand Down Expand Up @@ -1338,24 +1339,21 @@ static void extractqpairs(CURLU *uh, struct option *o)

static void qpair2query(CURLU *uh, struct option *o)
{
if(query_is_modified) {
int i;
char *nq = NULL;
for(i = 0; i<nqpairs; i++) {
char *oldnq = nq;
nq = curl_maprintf("%s%s%s", nq?nq:"",
(nq && *nq && *(qpairs[i].str))? o->qsep: "",
qpairs[i].str);
curl_free(oldnq);
}
if(nq) {
int rc = curl_url_set(uh, CURLUPART_QUERY, nq, 0);
if(rc)
trurl_warnf(o,
"internal problem: failed to store updated query in URL");
}
curl_free(nq);
int i;
char *nq = NULL;
for(i = 0; i<nqpairs; i++) {
char *oldnq = nq;
nq = curl_maprintf("%s%s%s", nq ? nq : "",
(nq && *nq && *(qpairs[i].str)) ? o->qsep : "",
qpairs[i].str);
curl_free(oldnq);
}
if(nq) {
int rc = curl_url_set(uh, CURLUPART_QUERY, nq, 0);
if(rc)
trurl_warnf(o, "internal problem: failed to store updated query in URL");
}
curl_free(nq);
}

/* sort case insensitively */
Expand All @@ -1375,18 +1373,20 @@ static int cmpfunc(const void *p1, const void *p2)
return 0;
}

static void sortquery(struct option *o)
static bool sortquery(struct option *o)
{
if(o->sort_query) {
/* not these two lists may no longer be the same order after the sort */
qsort(&qpairs[0], nqpairs, sizeof(struct string), cmpfunc);
qsort(&qpairsdec[0], nqpairs, sizeof(struct string), cmpfunc);
query_is_modified = true;
return true;
}
return false;
}

static void replace(struct option *o)
static bool replace(struct option *o)
{
bool query_is_modified = false;
struct curl_slist *node;
for(node = o->replace_list; node; node = node->next) {
struct string key;
Expand Down Expand Up @@ -1428,17 +1428,18 @@ static void replace(struct option *o)
qpairsdec[i].str = pdec->str;
free(pdec);
free(p);
replaced = true;
query_is_modified = replaced = true;
}

if(!replaced && o->force_replace) {
trurl_warnf(o, "key '%.*s' not in url, appending to query",
(int) (key.len),
key.str);
addqpair(key.str, strlen(key.str), o->jsonout);
query_is_modified = true;
}
query_is_modified = true;
}
return query_is_modified;
}
static CURLUcode seturl(struct option *o, CURLU *uh, const char *url)
{
Expand Down Expand Up @@ -1483,6 +1484,7 @@ static void singleurl(struct option *o,
do {
struct curl_slist *p;
bool url_is_invalid = false;
bool query_is_modified = false;
unsigned setmask = 0;

/* set everything */
Expand Down Expand Up @@ -1593,10 +1595,10 @@ static void singleurl(struct option *o,
extractqpairs(uh, o);

/* trim parts */
trim(o);
query_is_modified |= trim(o);

/* replace parts */
replace(o);
query_is_modified |= replace(o);

if(first_lap) {
/* append query segments */
Expand All @@ -1606,10 +1608,12 @@ static void singleurl(struct option *o,
}
}

sortquery(o);
/* sort query */
query_is_modified |= sortquery(o);

/* put the query back */
qpair2query(uh, o);
if(query_is_modified)
qpair2query(uh, o);

/* make sure the URL is still valid */
if(!url || o->redirect || o->set_list || o->append_path) {
Expand Down