Skip to content

Return correct download link for requests for non-standard crate names. #1758

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

Merged
merged 1 commit into from
Jun 10, 2019

Conversation

smarnach
Copy link
Contributor

@smarnach smarnach commented Jun 4, 2019

Fixes #1687.

Crates are uploaded to S3 under the name they were first published as, but the download endpoint always uses the name as written in the request. If these names differ in their use of dashes vs. underscores, the download endpoint returns an invalid link.

To avoid adding another database request for every single crate download, this fix changes the database request that updates the download count to also return the original crate name. We will need to confirm that the performance impact of this change is negligible.

Fixes rust-lang#1687.

Crates are uploaded to S3 under the name they were first published as, but the download endpoint
always uses the name as written in the request. If these names differ in their use of dashes
vs. underscores, the download endpoint returns an invalid link.

This fix changes the database request that updates the download count to also return the original
crate name.
@smarnach
Copy link
Contributor Author

smarnach commented Jun 6, 2019

Looks like the query plan remains virtually the same with this change. Old version:

                                                               QUERY PLAN                                                                
-----------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.15..6.69 rows=1 width=4) (actual time=0.087..0.090 rows=1 loops=1)
   ->  Nested Loop  (cost=0.15..13.23 rows=2 width=4) (actual time=0.084..0.084 rows=1 loops=1)
         Join Filter: (versions.crate_id = crates.id)
         ->  Index Scan using index_version_num on versions  (cost=0.15..12.18 rows=2 width=8) (actual time=0.035..0.036 rows=1 loops=1)
               Index Cond: ((num)::text = '0.1.0'::text)
         ->  Materialize  (cost=0.00..1.02 rows=1 width=4) (actual time=0.044..0.044 rows=1 loops=1)
               ->  Seq Scan on crates  (cost=0.00..1.02 rows=1 width=4) (actual time=0.039..0.039 rows=1 loops=1)
                     Filter: (replace(lower((name)::text), '-'::text, '_'::text) = 'crate_with_dashes'::text)
                     Rows Removed by Filter: 1
 Planning Time: 0.802 ms
 Execution Time: 0.151 ms

New version:

                                                               QUERY PLAN                                                                
-----------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.15..6.69 rows=1 width=36) (actual time=0.091..0.094 rows=1 loops=1)
   ->  Nested Loop  (cost=0.15..13.23 rows=2 width=36) (actual time=0.088..0.089 rows=1 loops=1)
         Join Filter: (versions.crate_id = crates.id)
         ->  Index Scan using index_version_num on versions  (cost=0.15..12.18 rows=2 width=8) (actual time=0.036..0.036 rows=1 loops=1)
               Index Cond: ((num)::text = '0.1.0'::text)
         ->  Materialize  (cost=0.00..1.02 rows=1 width=36) (actual time=0.047..0.048 rows=1 loops=1)
               ->  Seq Scan on crates  (cost=0.00..1.02 rows=1 width=36) (actual time=0.037..0.037 rows=1 loops=1)
                     Filter: (replace(lower((name)::text), '-'::text, '_'::text) = 'crate_with_dashes'::text)
                     Rows Removed by Filter: 1
 Planning Time: 0.645 ms
 Execution Time: 0.173 ms

@sgrif
Copy link
Contributor

sgrif commented Jun 10, 2019

@bors: r+

@bors
Copy link
Contributor

bors commented Jun 10, 2019

📌 Commit 076e7ce has been approved by sgrif

bors added a commit that referenced this pull request Jun 10, 2019
Return correct download link for requests for non-standard crate names.

Fixes #1687.

Crates are uploaded to S3 under the name they were first published as, but the download endpoint always uses the name as written in the request. If these names differ in their use of dashes vs. underscores, the download endpoint returns an invalid link.

To avoid adding another database request for every single crate download, this fix changes the database request that updates the download count to also return the original crate name. We will need to confirm that the performance impact of this change is negligible.
@bors
Copy link
Contributor

bors commented Jun 10, 2019

⌛ Testing commit 076e7ce with merge 09c3715...

@bors
Copy link
Contributor

bors commented Jun 10, 2019

☀️ Test successful - checks-travis
Approved by: sgrif
Pushing 09c3715 to master...

@bors bors merged commit 076e7ce into rust-lang:master Jun 10, 2019
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.

Wrong redirect URL given for a download with -/_ canonicalization
4 participants