Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repair the archive.org URL resolution #832

Merged
merged 3 commits into from
Oct 10, 2021

Conversation

pkgw
Copy link
Collaborator

@pkgw pkgw commented Oct 9, 2021

This should fix our ability to resolve the default bundle.

CC #765, #831

@codecov
Copy link

codecov bot commented Oct 9, 2021

Codecov Report

Merging #832 (68c5fc5) into master (574d08b) will increase coverage by 0.00%.
The diff coverage is 81.25%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #832   +/-   ##
=======================================
  Coverage   47.08%   47.08%           
=======================================
  Files         146      146           
  Lines       59489    59500   +11     
=======================================
+ Hits        28009    28016    +7     
- Misses      31480    31484    +4     
Impacted Files Coverage Δ
src/bin/tectonic/v2cli.rs 47.81% <0.00%> (ø)
crates/geturl/src/reqwest.rs 76.19% <75.00%> (-1.40%) ⬇️
src/unstable_opts.rs 66.66% <100.00%> (+2.22%) ⬆️
src/bin/tectonic/main.rs 75.00% <0.00%> (-2.50%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cde89ce...68c5fc5. Read the comment docs.

The archive.org PURL service made a recent change that broke our URL
resolution algorithm. The new redirect structure is:

```
https://archive.org/services/purl/net/pkgwpub/tectonic-default -> 307 Temporary redirect
https://purl.prod.archive.org/net/pkgwpub/tectonic-default -> 302 Found
https://ttassets.z13.web.core.windows.net/tlextras-2020.0r0.tar -> 200 OK
```

The problem is that we stop chasing redirects if the final path
component of the new URL doesn't contain a `.`, in an attempt to not
scan into things like S3 storage URLs. But this means that we stop at
the first URL, which is wrong. To fix this, we semi-hack the logic to
also follow the redirection if the filename of the new URL matches the
filename of the original URL.
This was surfaced by the recent archive.org issue, but doesn't actually
have anything to do with the proper solution of the issue. But while
we're at it, the 302 and 307 status codes are nearly semantically
equivalent, so if we accept 302 we should accept 307. Note that this fix
would have masked the archive.org issue ... things would have worked but
our URL resolution would have been giving improper results.
@pkgw pkgw merged commit 937f7db into tectonic-typesetting:master Oct 10, 2021
@pkgw pkgw deleted the repair-archive branch October 10, 2021 11:41
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.

2 participants