Bug 17785 - oai.pl returns wrong URLs under Plack
Summary: oai.pl returns wrong URLs under Plack
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-16 12:46 UTC by Marcel de Rooy
Modified: 2019-06-27 09:24 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 17785: oai.pl returns wrong URLs under Plack (9.88 KB, patch)
2016-12-20 15:23 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17785: Fix OAI-PMH's XSLT-generated URLs under Plack (10.01 KB, patch)
2016-12-21 13:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17785: Fix OAI-PMH's XSLT-generated URLs under Plack (10.08 KB, patch)
2016-12-27 15:04 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2016-12-16 12:46:31 UTC
oai.pl uses CGI's self_url
Under Plack this results in an URL like:
/opac/oai.pl?verb=ListRecords&resumptionToken=marcxml/100/1970-01-01T00:00:00Z/2016-12-16T23:59:59Z/
(This is the URL for the More button.)

Should we adjust the mounting point and ProxyPass statements?
Mount like /cgi-bin/koha/opac ?
Comment 1 Marcel de Rooy 2016-12-20 15:23:37 UTC
Created attachment 58303 [details] [review]
Bug 17785: oai.pl returns wrong URLs under Plack

Look at e.g. the URL for Show More at the end of the output of Records or
Identifiers. If you use Plack, you will see that it refers to
yourserver:/opac/oai.pl, which is not correct.

This is caused by using CGI's self_url in combination with script alias,
mounting point, etc. Note that we cannot solve this problem in the code of
Koha only. Since HTTP::OAI modules also call self_url, we still end up
with some wrong url's.

Instead of a larger architectural operation on Apache and Plack config files,
this patch adjusts the final xslt transformation on the OAI response.
It hardcodes the correct path only once, in a xslt variable. And replaces
all oai:OAI-PMH/oai:request/text() calls, containing wrong URLs, by this
variable.

Test plan:
Run oai.pl. Try the various verbs.
Verify that the URLs point to /cgi-bin/koha/oai.pl.
Comment 2 Marcel de Rooy 2016-12-20 15:25:13 UTC
Adding in cc some people involved in recent oai changes.
Comment 3 Tomás Cohen Arazi 2016-12-21 13:01:54 UTC
Created attachment 58338 [details] [review]
Bug 17785: Fix OAI-PMH's XSLT-generated URLs under Plack

Look at e.g. the URL for Show More at the end of the output of Records or
Identifiers. If you use Plack, you will see that it refers to
yourserver:/opac/oai.pl, which is not correct.

This is caused by using CGI's self_url in combination with script alias,
mounting point, etc. Note that we cannot solve this problem in the code of
Koha only. Since HTTP::OAI modules also call self_url, we still end up
with some wrong url's.

Instead of a larger architectural operation on Apache and Plack config files,
this patch adjusts the final xslt transformation on the OAI response.
It hardcodes the correct path only once, in a xslt variable. And replaces
all oai:OAI-PMH/oai:request/text() calls, containing wrong URLs, by this
variable.

Test plan:
Run oai.pl. Try the various verbs.
Verify that the URLs point to /cgi-bin/koha/oai.pl.

Edit: changed commit subject

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Works as expected. Good workaround until a definitive solution is implemented.
Comment 4 Jonathan Druart 2016-12-27 15:04:25 UTC
Created attachment 58435 [details] [review]
Bug 17785: Fix OAI-PMH's XSLT-generated URLs under Plack

Look at e.g. the URL for Show More at the end of the output of Records or
Identifiers. If you use Plack, you will see that it refers to
yourserver:/opac/oai.pl, which is not correct.

This is caused by using CGI's self_url in combination with script alias,
mounting point, etc. Note that we cannot solve this problem in the code of
Koha only. Since HTTP::OAI modules also call self_url, we still end up
with some wrong url's.

Instead of a larger architectural operation on Apache and Plack config files,
this patch adjusts the final xslt transformation on the OAI response.
It hardcodes the correct path only once, in a xslt variable. And replaces
all oai:OAI-PMH/oai:request/text() calls, containing wrong URLs, by this
variable.

Test plan:
Run oai.pl. Try the various verbs.
Verify that the URLs point to /cgi-bin/koha/oai.pl.

Edit: changed commit subject

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Works as expected. Good workaround until a definitive solution is implemented.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 5 Kyle M Hall 2016-12-28 13:36:31 UTC
Pushed to master for 17.05, thanks Marcel!
Comment 6 Katrin Fischer 2017-01-04 21:36:20 UTC
This patch has been pushed to 16.11.x and will be in 16.11.02.
Comment 7 Mason James 2017-01-27 02:41:05 UTC
Pushed to 16.05.x, for 16.05.08 release
Comment 8 Steven F. Schmieder 2017-03-11 13:01:50 UTC
I am having a similar problem that does appear to have been solved by the patch.  I running Debian Stretch RC2 with perl 5.24.1 and Koha 16.11.04.  Everythings works fine except the oai.pl script which returns an "internal server message".  As far as I can ascertain, the other scripts run fine.
Comment 9 Pablo AB 2018-07-22 18:18:47 UTC
As said [here](http://koha.1045719.n5.nabble.com/Validating-OAI-PMH-with-openarchives-org-tool-td5984438.html) and comment #5 even this bug was solved for 17.05+ I think still there.

1. Witg ByWater Solutions Koha Demo (17.11.05) https://catalog.bywatersolutions.com/cgi-bin/koha/oai.pl
2. Tested with validator: https://www.openarchives.org/Register/ValidateSite

Result: Get same two fails about baseURL and earliestDatestamp as with a 18.05 Koha.

Test2: With http://validator.oaipmh.com/
and Koha v17.11.06 https://demo.bibkat.no/cgi-bin/koha/oai.pl
return no errors about baseURL but it does about earliestDatestamp and content type.

Maybe passing this tests should be part of QA process.
Comment 10 Tomás Cohen Arazi 2018-07-22 19:05:49 UTC
(In reply to Pablo AB from comment #9)
> As said
> [here](http://koha.1045719.n5.nabble.com/Validating-OAI-PMH-with-
> openarchives-org-tool-td5984438.html) and comment #5 even this bug was
> solved for 17.05+ I think still there.
>
> Maybe passing this tests should be part of QA process.

Pablo, maybe there's a configuration issue with baseURL? And we could track down what's going on with earliestDatestamp. First check would be to know if there are records on the target system.
Comment 11 Tomás Cohen Arazi 2018-07-22 19:20:05 UTC
(In reply to Tomás Cohen Arazi from comment #10)
> (In reply to Pablo AB from comment #9)
> > As said
> > [here](http://koha.1045719.n5.nabble.com/Validating-OAI-PMH-with-
> > openarchives-org-tool-td5984438.html) and comment #5 even this bug was
> > solved for 17.05+ I think still there.
> >
> > Maybe passing this tests should be part of QA process.
> 
> Pablo, maybe there's a configuration issue with baseURL? And we could track
> down what's going on with earliestDatestamp. First check would be to know if
> there are records on the target system.

I reproduced two issues:

(a) Our fix for the XSLT masked the underlying bug (our code doesn't know about the proxypass we do, and then Plack answers what he knows, /opac/oai.pl).

(b) earliestDatestamp is read directly from the DB and not formatted as rfc3339 as it should).

The UTF-8 encoding issue deserves some research, because it is not clear to me that the response is incorrect (Content-Type: text/xml; charset=utf-8).
Comment 12 Tomás Cohen Arazi 2018-07-22 19:34:34 UTC
(In reply to Tomás Cohen Arazi from comment #11)
> The UTF-8 encoding issue deserves some research, because it is not clear to
> me that the response is incorrect (Content-Type: text/xml; charset=utf-8).

According to my interpretation of the spec, the test is not correct:

https://www.openarchives.org/OAI/openarchivesprotocol.html#HTTPResponseFormat
Comment 13 Pablo AB 2018-07-23 05:30:23 UTC
I found this issues:
a. "FAIL baseURL supplied [...]" from openarchives.org validation site
b. "FAIL Bad earliestDatestamp [...]"
c. validator.oaipmh.com warning about Content-Type
d. Koha XML output doesn't validate against OAI-PMH XML schema.

(In reply to Tomás Cohen Arazi from comment #11)
> (a) Our fix for the XSLT masked the underlying bug (our code doesn't know
> about the proxypass we do, and then Plack answers what he knows,
> /opac/oai.pl).

So this bug should be reopened? bibkat.no doesn't fail beacause it doesn't use Plack?

To test I found this command useful:
curl -s "https://sites/cgi-bin/koha/oai.pl?verb=Identify" | xmllint --format - | grep baseURL

> (b) earliestDatestamp is read directly from the DB and not formatted as
> rfc3339 as it should).

(b) responseDate is well formed, earliestDatestamp is not.
From XSD: "Tightened definition of UTCdatetimeType to enforce the restriction to UTC Z notation."
responseDate and expirationDate should be YYYY-MM-DDThh:mm:ssZ
https://www.openarchives.org/OAI/openarchivesprotocol.html#DatestampsResponses
So Koha should convert to this format what it gets from the DB.

(In reply to Tomás Cohen Arazi from comment #12)
> According to my interpretation of the spec, the test is not correct:
> https://www.openarchives.org/OAI/openarchivesprotocol.html#HTTPResponseFormat

(c) You are right, seems a problem of that validator. I'll contact site creator (Vangelis Banos).
Also acording to RFC7303 (9.2) which obsoletes RFC3023 (3).

On websites returning text/html validator.oaipmh.com oddly say they return a correct text/xml.

This problem is very common. A FOAF tested al the URLs from http://opendoar.org/OAIbaseURLs.php and get
575 text/xml
468 text/html
307 application/xml

(d) Koha XML sometimes invalid, tested with:
wget -q "https://www.openarchives.org/OAI/2.0/OAI-PMH.xsd"
wget -q "https://demo.bibkat.no/cgi-bin/koha/oai.pl?verb=ListSets" -O test.xml
xmllint --noout --schema OAI-PMH.xsd test.xml

- When no set is defined Koha incorrectly output <ListSets/>. A set (child element of ListSets) should always be there, minOccurs default value is 1. Otherwhise should be:
<error code="noSetHierarchy">No sets defined</error>, AFAIK even if tehere is no error.
- setDescription is not a string, is descriptionType ##other, "defined by a community" (?)
https://www.w3.org/TR/xmlschema-0/#ref34
https://www.w3.org/2005/07/xml-schema-patterns.html
https://www.openarchives.org/OAI/2.0/guidelines-repository.htm#setDescription
https://www.openarchives.org/OAI/openarchivesprotocol.html#Set  and 4.6 ListSets > Examples
Comment 14 Tomás Cohen Arazi 2018-07-23 13:58:19 UTC
(In reply to Pablo AB from comment #13)
> So this bug should be reopened? bibkat.no doesn't fail beacause it doesn't
> use Plack?

pablito, I think we need to focus on each issue in a separate place.

I would create an 'umbrella' bug (meaning it is not a real bug but a goal), and then file individual bugs for each alleged bug. Those tests you made should all be made against master. Not sure where, how. But we don't develop (usually) against stable branches. We develop against master, and the maintainers backport.

That said, file independent bugs, marking them as 'Blocks' the umbrella bug.

There's a situation with the HTTP::OAI library maintenance status that needs to be solved ASAP, but it might not be related to this bugs you highlight, as how the data comes from the DB for exposing it on the OAI-PMH front-end is a Koha business.

About the OAI-PMH spec requiring UTC, I think it is a plain wrong decision. Conversion can be done asynchronously on receiving the data (in the harvester), but in turn, it forces the provider to convert whatever data it has into UTC, causing an innecessary overhead.
Comment 15 Pablo AB 2018-07-23 19:53:14 UTC
(In reply to Tomás Cohen Arazi from comment #14)
> That said, file independent bugs, marking them as 'Blocks' the umbrella bug.

I created some bugs (28 so far)
(a) Bug 21104 - oai.pl returns wrong baseURL
(b) Bug 21105 - oai.pl returns invalid earliestDatestamp
(d) Bug 21106 - oai.pl XML output doesn't validate against OAI-PMH XML schema
All blocks 21102 and see also 17785.

About (c), I talk with Vangelis Banos and he already fix the Content-Type issue.