After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 641022 - SoupRequestData does not return decoded contents for data URLs
SoupRequestData does not return decoded contents for data URLs
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: Misc
2.33.x
Other Linux
: Normal normal
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2011-01-31 12:29 UTC by Sergio Villar
Modified: 2011-01-31 19:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (1.29 KB, patch)
2011-01-31 12:30 UTC, Sergio Villar
none Details | Review
soup-request-data: fix content-type decoding in non-base64 case (1.09 KB, patch)
2011-01-31 15:41 UTC, Dan Winship
none Details | Review
soup-request-data: fix content-type decoding in non-base64 case. v2 (1.38 KB, patch)
2011-01-31 19:05 UTC, Sergio Villar
accepted-commit_now Details | Review

Description Sergio Villar 2011-01-31 12:29:03 UTC
SoupRequestData is decoding data URLs but it's not returning the decoded data but the original one.
Comment 1 Sergio Villar 2011-01-31 12:30:34 UTC
Created attachment 179709 [details] [review]
Patch
Comment 2 Dan Winship 2011-01-31 15:41:56 UTC
Created attachment 179722 [details] [review]
soup-request-data: fix content-type decoding in non-base64 case

the original code made sense in webkit where it was using an in-place
URI-decoding method, but here, it's weird to assign the encoded value
to priv->content_type and then overwrite it. here's a counterpatch.

Does this break some WebKit test, or did you just notice the memory
leak?
Comment 3 Sergio Villar 2011-01-31 19:05:07 UTC
Created attachment 179737 [details] [review]
soup-request-data: fix content-type decoding in non-base64 case. v2

(In reply to comment #2)
> Created an attachment (id=179722) [details] [review]
> soup-request-data: fix content-type decoding in non-base64 case
> 
> the original code made sense in webkit where it was using an in-place
> URI-decoding method, but here, it's weird to assign the encoded value
> to priv->content_type and then overwrite it. here's a counterpatch.

Fair enough, but you're omitting the second part of the patch, the other soup_uri_decode that is not assigned to anything. Here you have my counter-counter-patch :)

> Does this break some WebKit test, or did you just notice the memory
> leak?

Yeah, basically most of tests with data URLs (after applying your webkitgtk+ patches) were not working, then I reviewed the libsoup code and detected the leak (and the bug).
Comment 4 Dan Winship 2011-01-31 19:23:19 UTC
Comment on attachment 179737 [details] [review]
soup-request-data: fix content-type decoding in non-base64 case. v2

> Fair enough, but you're omitting the second part of the patch

doh. Hence my confusion, since I couldn't see any webkit tests that had uri-encoded content-types, so just the first part shouldn't have mattered to that.

looks good. thanks.
Comment 5 Sergio Villar 2011-01-31 19:38:29 UTC
Committed ea5b7457