Skip to content

fix for double-url-encoded ":" in "hdl:". #9797 - #9835

Merged
kcondon merged 2 commits into
developfrom
9797-double-encoded-colon
Aug 29, 2023
Merged

fix for double-url-encoded ":" in "hdl:". #9797#9835
kcondon merged 2 commits into
developfrom
9797-double-encoded-colon

Conversation

@landreev

@landreev landreev commented Aug 28, 2023

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Copy-and-pasting the explanation of what's going on that I left in the issue earlier:

Here's what appears to be taking place:

Rather than trying to figure out why this has started happening under p6, I feel like we should just add a defensive '%3A' -> ':' substitution to the persistent id check. Hence this one line pr. It looks like this has already been happening once in a while - I see an occasional doi%253A resulting in a 404 in the prod. access logs (probably on account of bookmarked or harvested urls? - idk). So, would be a reasonable fix to add regardless.

The fact that this is happening because of the extra http: -> https: redirect is by itself weird; because that's done entirely under apache... so not immediately clear how the p6 upgrade would even affect that... But, once again, I'm not sure we want to spend much time figuring out the why part.

Which issue(s) this PR closes:

Closes #9797 maybe?

Special notes for your reviewer:

Suggestions on how to test this:

The obvious test is to make sure that urls like http://dataverse-internal.iq.harvard.edu/dataset.xhtml?persistentId=doi%3A10.70122/FK2/WPCTZB and http://dataverse-internal.iq.harvard.edu/api/datasets/export?exporter=schema.org&persistentId=doi%3A10.70122/FK2/WPCTZB are working. And maybe a more general check that there are no other links that may not be working depending on what port/protocol is used to get to the site. (?)

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:

Additional documentation:

@github-actions

This comment has been minimized.

@landreev
landreev marked this pull request as ready for review August 28, 2023 15:23
@landreev landreev added this to the 6.0 milestone Aug 28, 2023
@landreev

Copy link
Copy Markdown
Contributor Author

BTW, there's a chance that this is not unique to p6, but rather something uniquely misconfigured on dataverse-internal: It is running 5.14 under payara5/jdk11 at the moment, and the ids with %3A are bombing the same exact way when going through port 80: http://dataverse-internal.iq.harvard.edu/api/datasets/export?exporter=schema.org&persistentId=doi%3A10.70122/FK2/WPCTZB
... just something nobody noticed before. (does not happen in either prod. or on demo).

Just one extra reason to add this defensive line to the code and forget about it.

@pdurbin pdurbin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@landreev do you think any of these test failures are related to your pull request? https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-9835/1/testReport/

I sort of doubt it. I was otherwise about to approve this PR...

@landreev

Copy link
Copy Markdown
Contributor Author

Sorry for the delay. I can't imagine the one line in this PR being responsible for these failures either.
The actual test failures appear to be along the lines of

... Restricting files is not permitted on a public installation.

- so, something to do with the "public install" flag maybe?

@landreev

Copy link
Copy Markdown
Contributor Author

I synced the branch with develop, should fix the failing tests.

@kcondon kcondon self-assigned this Aug 29, 2023
@github-actions

Copy link
Copy Markdown

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:9797-double-encoded-colon
ghcr.io/gdcc/configbaker:9797-double-encoded-colon

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

@kcondon kcondon removed their assignment Aug 29, 2023
@kcondon
kcondon merged commit 9c078f0 into develop Aug 29, 2023
@kcondon
kcondon deleted the 9797-double-encoded-colon branch August 29, 2023 17:01
@kcondon kcondon self-assigned this Aug 29, 2023
@pdurbin pdurbin mentioned this pull request Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Payara6: URL encoding has changed so going through Apache or directly to 8080 can break things; export download, privateURL download.

4 participants