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

Fix #925 -- Remove project slug from file names #1000

Merged
merged 1 commit into from
Dec 21, 2016
Merged

Conversation

oliverroick
Copy link
Member

Proposed changes in this pull request

Fixes #925.

We get this error with Unicode file names (e.g. 東京財団-line.shp) because file names are encoded as a “surrogate escaped string” in a WSGI environment.

These are Unicode strings that cannot be encoded to a Unicode encoding because they are actually invalid. These strings are created by APIs that think an encoding is a specific one but cannot guarantee it because the underlying system does not fully enforce that. —Armin Ronacher: The Updated Guide to Unicode on Python

It is possible to prevent this error from happening by either ignoring or replacing invalid characters when encoding to UTF-8.

Given the example above (東京財団-line.shp), ignoring invalid characters results in the file being added as -line.shp to the download ZIP; replacing invalid characters results in ??????????-line.shp.

I decided to remove the project name from file names altogether to ensure consistent naming independent of the project name.

When should this PR be merged

Anytime.

Risks

None.

Follow up actions

None.

Copy link
Contributor

@seav seav left a comment

Choose a reason for hiding this comment

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

The code modifications are OK. But to really test that this bug fix works, we need to check it on the staging environment because this bug doesn't occur on the dev VM.

@amplifi amplifi requested review from amplifi and removed request for bjohare December 20, 2016 13:30
@seav
Copy link
Contributor

seav commented Dec 21, 2016

@amplifi, can we test this branch on staging to confirm the fix?

@amplifi amplifi merged commit df50166 into master Dec 21, 2016
@amplifi amplifi deleted the bugfix/#925 branch December 21, 2016 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants