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 MIME type checks #231

Merged
merged 1 commit into from Feb 28, 2019

Conversation

bendemboski
Copy link
Member

Using a regular expression to check MIME types was problematic and unnecessary. Problematic because it wasn't escaping characters, so for example, image/svg+xml would not match because the + was interpreted as a 1-or-more modifier on the g. Unnecessary because the accept attribute only accepts a very limited set of clearly enumerated type wildcards (see https://html.spec.whatwg.org/multipage/input.html#file-upload-state-(type=file)).

So, match more tightly and correctly doing string comparisons, and add some unit tests for this (and to verify case-insensitivity, which was missing for mime types).

Using a regular expression to check MIME types was problematic and unnecessary. Problematic because it wasn't escaping characters, so for example, `image/svg+xml` would not match because the `+` was interpreted as a 1-or-more modifier on the `g`. Unnecessary because the accept attribute only accepts a very limited set of clearly enumerated type wildcards (see https://html.spec.whatwg.org/multipage/input.html#file-upload-state-(type=file)).

So, match more tightly and correctly doing string comparisons, and add some unit tests for this (and to verify case-insensitivity, which was missing for mime types).
@Alonski
Copy link
Member

Alonski commented Feb 26, 2019

Hmm Ember Beta and Ember Canary are failing builds unrelated to this PR. If you could also help out with getting these tests passing that would be awesome. If not I will hopefully get to it this week or next week :)

@bendemboski
Copy link
Member Author

I think I should be able to get to that in later today.

@Alonski
Copy link
Member

Alonski commented Feb 26, 2019

Awesome @bendemboski thanks!

@bendemboski
Copy link
Member Author

Okay, investigated a bit, and the failure is because ember-beta and ember-canary broke ember-concurrency (which is used by ember-addon-docs). It sounds like the PR reverting the breaking changes will be released to beta today or tomorrow (see here), and when that happens, the build will magically start passing 😄

When that gets released I'll poke this branch to get it rerun the build and ping you once it's ready and passing.

@Alonski
Copy link
Member

Alonski commented Feb 26, 2019

Awesome detective work! Let's wait for that and once tests are passing let's get this stuff merged!

@bendemboski
Copy link
Member Author

@Alonski build passing now!

@Alonski Alonski merged commit 8cb4b4d into adopted-ember-addons:latest Feb 28, 2019
@jelhan jelhan added the bug label Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants