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 file:// URI parsing #1601

Merged
merged 1 commit into from
Jan 30, 2023
Merged

Fix file:// URI parsing #1601

merged 1 commit into from
Jan 30, 2023

Conversation

TheKeeperOfPie
Copy link
Contributor

A previous change to fix #1344 to handle # characters in literal no-scheme paths broke file:// scheme URI parsing by including the entire literal URI in the file path, which breaks encoded paths (like those with spaces) and URIs with non-path data (such as query parameters).

This partially reverts the prior fix, so that # is not supported in file:// URIs, requiring the user to either use a no-scheme URI or encode the path, as # is reserved in URIs for fragment declarations.

Fixes: #1513

Test: FileUriMapperTest#ignoresAfterPathCorrectly
Test: FileUriMapperTest#decodesEncodedPath

A previous change to fix #1344 to handle # characters in literal no-scheme
paths broke file:// scheme URI parsing by including the entire literal URI in
the file path, which breaks encoded paths (like those with spaces) and URIs
with non-path data (such as query parameters).

This partially reverts the prior fix, so that # is not supported in file://
URIs, requiring the user to either use a no-scheme URI or encode the path,
as # is reserved in URIs for fragment declarations.

Fixes: #1513

Test: FileUriMapperTest#ignoresAfterPathCorrectly
Test: FileUriMapperTest#decodesEncodedPath
Copy link
Member

@colinrtwhite colinrtwhite left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this and adding regression tests! It's too bad we can't support # in file URIs, but makes sense given the spec (as you mentioned).

@colinrtwhite colinrtwhite merged commit cf296dd into coil-kt:main Jan 30, 2023
colinrtwhite added a commit that referenced this pull request Feb 21, 2023
* main:
  Update the baseline files. (#1625)
  Always invoke mkdir when creating DiskCache. (#1626)
  Update dependency gradle to v8.0.1 (#1623)
  Fix DiskCache maxSize calculation (#1619) (#1620)
  Update plugin binaryCompatibility to v0.13.0 (#1621)
  Update dependency gradle to v8 (#1618)
  Update dependency androidx.profileinstaller:profileinstaller to v1.3.0-beta01 (#1613)
  Update Kotlin to 1.8.10. (#1614)
  Update dependency androidx.exifinterface:exifinterface to v1.3.6 (#1612)
  Update dependency androidx.appcompat:appcompat-resources to v1.6.1 (#1611)
  Migrate to collectStableBaselineProfile. (#1607)
  Update dependency com.android.tools.build:gradle to v7.4.1 (#1608)
  Fix file:// URI parsing (#1601)
  Update dependency com.vanniktech:gradle-maven-publish-plugin to v0.24.0 (#1603)
  Update dependency com.google.android.material:material to v1.8.0 (#1600)
  Update Kotlin to 1.8. (#1597)
  Update dependency com.vanniktech:gradle-maven-publish-plugin to v0.23.2 (#1596)
  Update dependency com.android.tools.build:gradle to v7.4.0 (#1594)
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.

Version 2.2.2 uri load bug File name contain '#' load error.
2 participants