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.

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

curl_url query urlencoding ignores + character #6086

Closed
Rattenhirn opened this issue Oct 15, 2020 · 3 comments
Closed

curl_url query urlencoding ignores + character #6086

Rattenhirn opened this issue Oct 15, 2020 · 3 comments
Labels

Comments

@Rattenhirn
Copy link

@Rattenhirn Rattenhirn commented Oct 15, 2020

I did this

{
	Curl_URL* url = curl_url();

	const char* inQuery = "hash=Al2cO3tDkcDZ3EWE5Lh+LX8TPHs=";
	char* outQuery;

	curl_url_set(url, CURLUPART_QUERY, inQuery, CURLU_URLENCODE);
	curl_url_get(url, CURLUPART_QUERY, &outQuery, CURLU_URLDECODE);

	printf("In Query:  %s\n", inQuery);
	printf("Out Query: %s\n", outQuery);

	curl_free(outQuery);
	curl_url_cleanup(url);
}

and got:

In Query:  hash=Al2cO3tDkcDZ3EWE5Lh+LX8TPHs=
Out Query: hash=Al2cO3tDkcDZ3EWE5Lh LX8TPHs=

Note that the + is gone.

I expected the following

In Query:  hash=Al2cO3tDkcDZ3EWE5Lh+LX8TPHs=
Out Query: hash=Al2cO3tDkcDZ3EWE5Lh+LX8TPHs=

I debugged into it, and it seems that there is conflict between replacing spaces with pluses, and url encoding those pluses.
Moving the plus encode part in curl_url_set below the url encode part, and removing the exception for pluses in the url encode part will probably fix the issue in this case. However, it make break other things, as this function is very semantically loaded. :)

curl/libcurl version

Built from curl-7.72.0 tar

operating system

Windows

bagder added a commit that referenced this issue Oct 15, 2020
... when asked to with URL CURLU_URLENCODE.

Extended test 1560 to verify.
Reported-by: Dietmar Hauser
Fixes #6086
@bagder
Copy link
Member

@bagder bagder commented Oct 15, 2020

I could reproduce and #6087 fixes it for me. I also added a test to verify but it would be good if you could try out the fix for your use case!

@Rattenhirn
Copy link
Author

@Rattenhirn Rattenhirn commented Oct 15, 2020

Fix verified!

@bagder
Copy link
Member

@bagder bagder commented Oct 15, 2020

Excellent, thanks!

@bagder bagder closed this in b7ea3d2 Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.