Skip to content

fix: handle signed urls#256

Merged
bdon merged 2 commits intoprotomaps:mainfrom
egorbn:handle-signed-urls
Dec 4, 2025
Merged

fix: handle signed urls#256
bdon merged 2 commits intoprotomaps:mainfrom
egorbn:handle-signed-urls

Conversation

@egorbn
Copy link
Contributor

@egorbn egorbn commented Nov 19, 2025

Background

Currently, NormalizeBucketKey discards the query string from http:// urls.

This leads to errors when trying to pmtiles show http://some.bucket.com/the_data.pmtiles?signature=blablabl:

Failed to show archive, Failed to create range reader for the_data.pmtiles, HTTP error: 403

Changes

  • Keep the query in the file target passed to the NewRangeReader
  • Added tests
    • PLEASE VERIFY! -- test is AI generated

Related issues

#255

@egorbn
Copy link
Contributor Author

egorbn commented Nov 26, 2025

Bumping this bug fix @bdon @roblabs

assert.Nil(t, err)
}

func TestHttpBucketRequestWithQuery(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

@egorbn This test does not fail when the original code is left as-is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the test are passing both in on github and when running go test ./pmtiles.
image

Am I missing something?

The publish step is failing because it's complaining about a password:
image

What do I need to do to fix that?

Copy link
Member

Choose a reason for hiding this comment

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

The publish step is failing because it's complaining about a password

Don't worry about this, it only matters when running on main

All the test are passing both in on github and when running

The test does not fail before the code change, so are you sure this is an effective test?

@bdon bdon merged commit f2434a5 into protomaps:main Dec 4, 2025
3 of 4 checks passed
bdon added a commit that referenced this pull request Dec 4, 2025
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