refactor: replace filterFlags with mime.ParseMediaType - #4758
refactor: replace filterFlags with mime.ParseMediaType#4758wanghaolong613 wants to merge 1 commit into
Conversation
rashmi-tondare
left a comment
There was a problem hiding this comment.
Good direction since mime.ParseMediaType fixes real defects in the old scan (leading whitespace, case sensitivity). One thing to check though: ParseMediaType can return a non-empty mediatype and a non-nil error (ErrInvalidMediaParameter) for headers with a valid base type but a malformed param. The current err != nil fallback doesn't handle that correctly.
Also, could use some table-driven tests in utils_test.go for the cases this is meant to fix (whitespace, case, missing space before ;), plus a couple of malformed-header cases to pin down the fallback behavior.
OK, I'll fix it |
9cb41f0 to
1fe89c9
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4758 +/- ##
==========================================
- Coverage 99.21% 98.33% -0.89%
==========================================
Files 42 48 +6
Lines 3182 3174 -8
==========================================
- Hits 3157 3121 -36
- Misses 17 43 +26
- Partials 8 10 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1fe89c9 to
8060370
Compare
|
Changes look good to me! Just one small thing, would be nice to add the pull request template in the PR description and tick off appropriate items from the checklist. |
Thanks for the feedback! I'll add the PR template to the description and tick off the relevant checklist items right away. |
rashmi-tondare
left a comment
There was a problem hiding this comment.
LGTM!
@appleboy this fix looks sound to me. It contains tests for the fixed issue as well and I believe would be a good addition to the lib.
Pull Request Checklist
Please ensure your pull request meets the following requirements:
masterbranch.docs/doc.md.Changes
Replace handmade
filterFlagssimple string truncation logic with standard librarymime.ParseMediaTypeto parse Content-Type header.Purpose
Testing
Added test cases to verify parsing behavior for: