quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.79k stars 2.68k forks source link

application/x-www-form-urlencoded "+" instead of %20 #43281

Closed kzmKZuzmo closed 1 month ago

kzmKZuzmo commented 1 month ago

Describe the bug

sniffed from fidller

POST https://test.com/identity/connect/token HTTP/1.1
Content-Type: application/x-www-form-urlencoded
User-Agent: Quarkus REST Client
content-length: 218
host: test.com

password=password123456&
grant_type=password&
scope=openid+profile+api+email&
subdomain=subdomain123&
client_secret=SidA&
client_id=0a2df0f5-1234-4c0d-a76b-c72f128b7c8c&
username=pass@pass.net
@RestForm("client_secret")
    private String clientSecret;
    @RestForm("grant_type")
    private String grantType;
    @RestForm("client_id")
    private String clientId;
    @RestForm("scope")
    private String scope;
    @RestForm("username")
    private String username;
    @RestForm("subdomain")
    private String subdomain;
    @RestForm("password")
    private String password;
  getters setters

@RegisterRestClient(configKey = "TokenIntegration-api")
@Path("/identity/connect/token")
public interface TokenIntegration {

    @POST
    @Consumes({MediaType.APPLICATION_FORM_URLENCODED})
    @Produces(MediaType.APPLICATION_JSON)
    getToken(TokenRequest TokenRequest);
}

 @Inject
    @RestClient
    TokenIntegration tokenIntegration;

 String scope = "openid profile api email";
 data.addSCope(scope)
 tokenIntegration.getToken(data);

Expected behavior

scope=openid%20profile%20api%20email&

Actual behavior

scope=openid+profile+api+email&

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

17

Quarkus version or git rev

3.14.3

Build tool (ie. output of mvnw --version or gradlew --version)

3.9.9

Additional information

No response

geoand commented 1 month ago

Thanks for reporting.

Is there any chance you can put that code in a sample project we can then just launch and see the problem in action?

quarkus-bot[bot] commented 1 month ago

/cc @cescoffier (rest-client)

kzmKZuzmo commented 1 month ago

@geoand check this I Extract a create som example

GET http://localhost:8080/api/test project here demo.zip image

geoand commented 1 month ago

I don't the problem described in the title when running the sample. What am I missing here?

kzmKZuzmo commented 1 month ago

@geoand

when request is sended to the API with Content-Type: application/x-www-form-urlencoded and you have key for example "scope" and value for this key is "something test testx example" (delimiter is "space") this space should be encoded with %20 and not with "+" should be looks like something%20test%20testx%20example not like this something+test+testx+example

geoand commented 1 month ago

As far as I can tell from all the online material I read, + is a valid encoding for a space in form params, so I see no compelling reason to change this, but I could be persuaded otherwise.

@FroMage @cescoffier WDYT?

sberyozkin commented 1 month ago

This one should probably be configurable, we did have it configurable in the former project I worked on. Some consumers don't decode + as space, even though + can indeed be a valid encoding of

geoand commented 1 month ago

Sure, but until I hear from someone trying to interact with a system that doesn't accept + (and details of what that system is), I don't see a reason to add the complexity.

sberyozkin commented 1 month ago

Yeah, good point; may be worth enforcing it as %20, as making it configurable is indeed an extra problem, which can be important only if some server wants to treat + as +. Having %20 is always correct for spaces.

geoand commented 1 month ago

Having %20 is always correct for spaces.

Right, that's what I have found as well

FroMage commented 1 month ago

https://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.1 (HTML 4 spec, way outdated) says spaces are encoded as +.

HTML5 points ultimately to https://url.spec.whatwg.org/#application/x-www-form-urlencoded which says that on decoding + is decoded to space (guessing for compat reasons), and on encoding, spae is encoded as + while + is percent-encoded

That seems to be the official encoding of that horrible charset, as mentioned by the spec:

The application/x-www-form-urlencoded format is in many ways an aberrant monstrosity, the result of many years of implementation accidents and compromises leading to a set of requirements necessary for interoperability, but in no way representing good design practices. In particular, readers are cautioned to pay close attention to the twisted details involving repeated (and in some cases nested) conversions between character encodings and byte sequences. Unfortunately the format is in widespread use due to the prevalence of HTML forms

geoand commented 1 month ago

Thanks for the information @FroMage!

kzmKZuzmo commented 1 month ago

Sure, but until I hear from someone trying to interact with a system that doesn't accept + (and details of what that system is), I don't see a reason to add the complexity.

Im trying to integrate to the system, and they dont accent that "+" so this is the reason why Im writing here.

geoand commented 1 month ago

What system is that? Are you sure it's not configurable?

geoand commented 1 month ago

Closing for lack of feedback