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 653980 - libcurl file-uri backend does not work with ftp
libcurl file-uri backend does not work with ftp
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
git master
Other All
: Normal normal
: 2.8
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2011-07-05 00:52 UTC by speps
Modified: 2015-11-29 14:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add ftp gopher and curl 7.21.7 support for libcurl file-uri backend (944 bytes, application/octet-stream)
2011-07-05 00:52 UTC, speps
  Details
Better patch for a per protocol success checking (2.15 KB, patch)
2011-07-05 14:13 UTC, speps
none Details | Review
Use a more robust g_ascii_strncasecmp, better response check (2.41 KB, patch)
2011-07-05 17:34 UTC, speps
none Details | Review
Same patch with no gopher support (2.03 KB, patch)
2011-07-05 17:42 UTC, speps
none Details | Review

Description speps 2011-07-05 00:52:54 UTC
Created attachment 191262 [details]
add ftp gopher and curl 7.21.7 support for libcurl file-uri backend

uri-backend-libcurl actually validate for a correct download
by checking the response code to be equal to "200".
This is correct for http protocol, but that's not the same for ftp.
Ftp response code needs to be "226" to be successful.
The attached patch add support for ftp and gopher protocol
by allowing "226" (ftp) and "0" (gopher) to be considered successful.
Also this adds support for curl 7.21.7 (does not provide anymore "types.h")
This patch should be enough strong, btw maybe a rare matching "226" code
for http could occur and give a false positive.
Comment 1 Mikael Magnusson 2011-07-05 01:14:06 UTC
Seems a bit hacky, isn't there any better way to do this?
Comment 2 speps 2011-07-05 14:13:22 UTC
Created attachment 191322 [details] [review]
Better patch for a per protocol success checking

New patch classify the effective url to act
a per protocol response_code success check.
Comment 3 Mikael Magnusson 2011-07-05 14:30:34 UTC
That looks better I suppose, should we check for https too though? And I think you want a prefix match, strcasestr will find the string anywhere in eff_url, which I guess is unlikely to happen given the order of the checks, but it would still be better to do a prefix match. http://developer.gnome.org/glib/2.28/glib-String-Utility-Functions.html#g-ascii-strncasecmp should do the trick.

(and out of curiosity, do you open image files over gopher often? :)
Comment 4 speps 2011-07-05 17:34:42 UTC
Created attachment 191340 [details] [review]
Use a more robust g_ascii_strncasecmp, better response check

Thanks, this should be stronger.
About gopher, yep actually i never use it. ;)
There is an open bug dealing about this on Archlinux
and a user made a request about gopher support in gimp [1]

I have to admit gopher support is not well handled in curl too.
There's no a simple way to actually control a gopher link validity,
response_code defaults on 0 on success and fail.
curl_easy_perform results positive even if the file is not found on server.

With this patch gopher works fine when the uri is correct,
otherwise it would fail on successive file opening (zero lenght file).

While a better patch would not come out
and this one is not stable enough for you,
gopher support can be omitted.

[1] https://bugs.archlinux.org/task/12321#comment79187
Comment 5 speps 2011-07-05 17:42:54 UTC
Created attachment 191342 [details] [review]
Same patch with no gopher support
Comment 6 Mikael Magnusson 2011-07-05 17:56:58 UTC
Thanks, I've committed the version with gopher support, since it wasn't really that intrusive anyway :).

commit 039d4636f84ade6387216e347f5af597f43b99fc                                                           
Author: Mikael Magnusson <mikachu@src.gnome.org>
Date:   Tue Jul 5 19:52:33 2011 +0200

    plug-ins: add ftp and gopher support to curl uri backend

commit b8740d285d4689fbacfa8ba8f83649f593de2e8b
Author: Mikael Magnusson <mikachu@src.gnome.org>
Date:   Tue Jul 5 19:53:22 2011 +0200

    plug-ins: remove curl/types.h include in curl uri backend which was removed in 7.21.7