Bug 14530 - 856$u URL checker ( check-url.pl ) complains about ftp based URLs
Summary: 856$u URL checker ( check-url.pl ) complains about ftp based URLs
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-14 13:22 UTC by Kyle M Hall
Modified: 2016-05-02 13:09 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 14530 - 856$u URL checker ( check-url.pl ) complains about ftp based URLs (2.17 KB, patch)
2015-07-16 13:39 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2015-07-14 13:22:00 UTC
The script check-url.pl assumes that any URL not prefixed with http must be a relative URL. This is problematic as the URL could be prefixed with ftp as well.

Any 856$u using an ftp url is given the error "400 URL Must be absolute".
Comment 1 Kyle M Hall 2015-07-16 13:39:24 UTC
Created attachment 41026 [details] [review]
Bug 14530 - 856$u URL checker ( check-url.pl ) complains about ftp based URLs

The script check-url.pl assumes that any URL not prefixed with http must
be a relative URL. This is problematic as the URL could be prefixed with
ftp as well.

Any 856$u using an ftp url is given the error "400 URL Must be
absolute".

Since HTTP::Request cannot handle ftp urls, we should just skip them.

I also noticed that check-url-quick.pl checks for prefixes with
case insensitivity. That seems like a good idea so I've added
that to check-url.pl

Test Plan:
1) Apply this patch
2) Find or create a record with an ftp url in 865$u
3) Run check-url.pl
4) Note the ftp url is skipped
5) Run check-url-quick.pl
6) Note the ftp url is skipped
Comment 2 Mark Tompsett 2015-07-25 18:43:34 UTC
Actually, couldn't you put an http URL for an ftp proxy into host?

./misc/cronjobs/check-url.pl --host=http://this.site.proxies.ftp.requests

then it would attempt to go to http://this.site.proxies.ftp.requests/ftp://whatever/whereever/whatever.foo

Personally, I'd prefer to see a re-write to allow for other protocols (bring back gopher! :) ).

Because skipping the ftp URLs kind of defeats the purpose of the script which is to validate that the URLs exist. This could be a significant problem if a large portion of URLs are not http.
Comment 3 Mark Tompsett 2015-07-26 00:27:10 UTC
Also, quick says it replaces the non-quick version. Perhaps we should delete the non-quick version?
Comment 4 Kyle M Hall 2015-08-14 23:02:25 UTC
(In reply to M. Tompsett from comment #2)
> Actually, couldn't you put an http URL for an ftp proxy into host?
> 
> ./misc/cronjobs/check-url.pl --host=http://this.site.proxies.ftp.requests
> 
> then it would attempt to go to
> http://this.site.proxies.ftp.requests/ftp://whatever/whereever/whatever.foo
> 
> Personally, I'd prefer to see a re-write to allow for other protocols (bring
> back gopher! :) ).
> 
> Because skipping the ftp URLs kind of defeats the purpose of the script
> which is to validate that the URLs exist. This could be a significant
> problem if a large portion of URLs are not http.

That sounds reasonable, but not everyone will want to go to the trouble of setting that up. As a middle ground, I think adding a --skip-ftp parameter should suffice.
Comment 5 Kyle M Hall 2015-08-14 23:03:24 UTC
(In reply to M. Tompsett from comment #3)
> Also, quick says it replaces the non-quick version. Perhaps we should delete
> the non-quick version?

That sounds good to me. Would you want to file a separate bug report for that? If that bug/patch makes it into master before this one, I'll just have to update it.

Thanks!
Comment 6 Mark Tompsett 2015-08-15 04:36:22 UTC
BTW, I was bored a while back, and actually wrote a forking LWP version. I'll attach it later, once I actually test it in place. :)
Comment 7 Jesse Lambertson 2016-05-02 13:09:38 UTC
This is mostly a question about URL checking in general.

This script checks for good links in 856 $ u?

Cab it be run anywhere while logged into the server, or does it need to be run from a specific part in the file system?

Just want to make sure.
Thank you,

Jesse