Skip to content

Conversation

Copy link

Copilot AI commented Dec 2, 2025

  • Implement to_unsafe_string() method for URLSearchParams
  • Add C API function ada_search_params_to_unsafe_string
  • Add tests for both C++ and C APIs
  • Rename to_raw_string to to_unsafe_string per reviewer feedback
  • Update documentation to clarify the non-standard compliant nature of the method
  • Fix clang-format issues
  • Remove accidentally committed build artifacts from git

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@anonrig
Copy link
Member

anonrig commented Dec 2, 2025

@copilot can you also make sure the documentation reflects this:

/**

  • Returns a serialized query string without normalizing the key-value pairs.
  • Unlike to_string(), this method does not apply additional transformations
  • to the percent-encoded output. The result is not standard compliant and
  • is therefore unsafe.
    */

Co-authored-by: anonrig <1935246+anonrig@users.noreply.github.com>
@anonrig
Copy link
Member

anonrig commented Dec 2, 2025

@copilot make sure lint and format ci job passes

Co-authored-by: anonrig <1935246+anonrig@users.noreply.github.com>
Copilot AI changed the title [WIP] Add URLSearchParams::to_raw_string() method add URLSearchParams::to_unsafe_string() method Dec 2, 2025
Copilot AI requested a review from anonrig December 2, 2025 12:53
Copilot finished work on behalf of anonrig December 2, 2025 12:53
Co-authored-by: anonrig <1935246+anonrig@users.noreply.github.com>
Copy link
Author

Copilot AI commented Dec 2, 2025

@copilot make sure lint and format ci job passes

Fixed clang-format issues in 94c3a8f. The lint/format CI should pass now.

@copilot can you also make sure the documentation reflects this:

/**...

The documentation already reflects that exact format - lines 103-108 in include/ada/url_search_params.h match the requested comment.

Copilot finished work on behalf of anonrig December 2, 2025 12:59
@bungle
Copy link

bungle commented Dec 2, 2025

I am not against this change, but I think it will not solve the issue in its current form:
#1022

See: #1023 (comment)

The expected use from our side is to feed ada search params with some standard (w3c/rfc/mixed) or with small amount of deviation from them, and then do non-destructive changes to it, for example removal of a single parameter (meaning rest should be treated as normal, if someone replaces existing parameter, it is fine to do it normal ada w3c way, but again, not touch any other parameter). The problem is that once query is fed to ada, it gets decoded and that process loses original information as multiple values may decode to same decoded value. With some of our customers this causes unexpected behavior. The first issue we got reported was that when we removed one parameter, the %20 turned to + in rest of the parameters and their upstream was not able to handle it. Second report came with retaining comma , in query value, but ada forcibly percent encodes it. It is not an issue with Kong, but with some upstream servers that we don't have control. Yes, we have told that we do this according to standards, but sometimes you may need to bend (easier to make proxy behaving according to user/customer preferences rather than asking them to fix their own code - for them it is kong that is not behaving nicely :-( ). These are very edgy cases, I admit.

@@ -0,0 +1 @@
. No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

don't push this file

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants