Skip to content

urlapi: preserve empty markers in relative URLs - #22298

Closed
Alb3e3 wants to merge 2 commits into
curl:masterfrom
Alb3e3:fix-url-empty-query-relative
Closed

urlapi: preserve empty markers in relative URLs#22298
Alb3e3 wants to merge 2 commits into
curl:masterfrom
Alb3e3:fix-url-empty-query-relative

Conversation

@Alb3e3

@Alb3e3 Alb3e3 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What

Relative URL resolution currently loses explicit empty query/fragment markers when those markers are needed to compute the splice point.

Example before this patch:

curl_url_set(u, CURLUPART_URL, "http://host/path?#", 0);
curl_url_set(u, CURLUPART_URL, "#new", 0);
curl_url_get(u, CURLUPART_URL, &url, CURLU_GET_EMPTY);

Expected:

http://host/path?#new

Actual before this patch:

http://host/path#new

The empty query marker is significant when the caller asks for empty components with CURLU_GET_EMPTY.

This patch fixes the relative-resolution path by:

  • preserving empty query/fragment separators when serializing the existing base URL for relative resolution
  • using query_present and fragment_present when choosing the cutoff point, instead of only checking for non-empty query/fragment strings
  • adding focused unit1675 coverage for fragment-only, query-only, and path-relative references across explicit empty query/fragment markers

Tests

cmake --build _build --target units -j2 && ./_build/tests/unit/units unit1675
perl ../../tests/runtests.pl -c ../src/curl -a -n 1675

I also checked the public URL API behavior with a small local probe for the examples above.

Disclosure: AI assistance was used while inspecting and drafting this patch; the behavior and focused tests listed above were verified locally.

@github-actions github-actions Bot added the tests label Jul 10, 2026

@bagder bagder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks like a perfectly fine change, thanks! I would however prefer the test code to be added to test 1560 instead (lib1560.c) since it can be a libtest, not a unit test, and 1560 is already doing most of the URL API testing.

@Alb3e3

Alb3e3 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Moved the URL API regression coverage into lib1560: the existing #, ?, and ?# cases now assert explicit markers via CURLU_GET_EMPTY, and all five relative-resolution cases from unit1675 are covered in set_url_list. Removed those public-API cases from unit1675. Focused lib1560 and unit1675 runs pass.

@bagder bagder closed this in c221469 Jul 21, 2026
@bagder

bagder commented Jul 21, 2026

Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants