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

Support files without extensions in importFromUrl #1873

Merged
merged 11 commits into from
Jun 17, 2023

Conversation

wladif
Copy link
Contributor

@wladif wladif commented Jun 9, 2023

This fixes the enhancement #1652.
It extends the importFromUrl logic to support importing of images from URLs without any extension.

Caveats:

  1. the constant SAMPLE_DOWNLOAD_TIFF_WITHOUT_EXTENSION needs to be updated to point to Lychee repo once this PR is merged. I'll do in a future PR.
  2. Unfortunately, Github sets the content type to application/octet-stream when file doesn't have an extension, this means that the branch elseif (isset($originalMimeType) && self::isSupportedMimeType($originalMimeType)) is not covered by current tests. If you know a provider/way to host the sample files (mongolia & tiff) without extension so that the web server will set the Content Type correctly (e.g. image/jpeg), please let me know and I'll add the tests. Thanks!

@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Merging #1873 (5467065) into master (1a0b831) will decrease coverage by 0.48%.
The diff coverage is 83.78%.

Additional details and impacted files

@ildyria ildyria self-assigned this Jun 11, 2023
@ildyria
Copy link
Member

ildyria commented Jun 13, 2023

To make it easier for you in the future, I suggest when you want to add a feature to create a new branch instead of doing it on master on your side :)
A bit of doc is here: https://lycheeorg.github.io/docs/contributions.html#how-to-properly-submit-a-pull-request-to-lychee

@wladif
Copy link
Contributor Author

wladif commented Jun 15, 2023

Cool! Thanks! Is there any issue that I still need to address?

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