Skip to content

Conversation

@cho8
Copy link
Contributor

@cho8 cho8 commented Oct 28, 2019

Issue #60 -- Increased Code Coverage.
Fixed a bug where file mime returned always included delimiter.
Refactored and added unit tests.

@cho8 cho8 changed the title ServiceInterceptor - Unit Tests and Bug Fix SerializeInterceptor - Unit Tests and Bug Fix Oct 28, 2019
@cho8 cho8 changed the title SerializeInterceptor - Unit Tests and Bug Fix SerializeInterceptor - Unit Tests Coverage and Bug Fix Oct 28, 2019
@coveralls
Copy link

coveralls commented Oct 28, 2019

Coverage Status

Coverage increased (+0.2%) to 44.821% when pulling ba0fbd7 on cho8:serialize-interceptor-tests into eaeb2e6 on intuit:develop.

@cho8 cho8 changed the title SerializeInterceptor - Unit Tests Coverage and Bug Fix #hacktoberfest SerializeInterceptor - Unit Tests Coverage and Bug Fix Oct 30, 2019
@cho8
Copy link
Contributor Author

cho8 commented Oct 31, 2019

@diana-derose Can I get some feedback on this PR?

@diana-derose
Copy link
Collaborator

@cho8 - Could you explain what was the bug in the getMime method?

@cho8
Copy link
Contributor Author

cho8 commented Oct 31, 2019

@diana-derose The getMime method was returning the delimiter when it parses for the file mime.

The beginIndex in substring(int beginIndex, int endIndex) is inclusive. In the case of getMime, this means it will include the delimiter when it parses out the mime on line 205:
return name.substring(name.lastIndexOf(delimiter), name.length());

e.g. filename.jpg would result in .jpg instead of just jpg

If the file uploaded is an image, the image content would never be retrieved as intended since the returned mime would never match a known image type from line 49
private static final String[] IMAGE_MIMES = {"jpg", "jpeg", "tif", "tiff", "gif", "png", "bmp"};

This PR fixes this issue by incrementing the beginIndex by 1
return name.substring(name.lastIndexOf(delimiter) + 1, name.length());

@cho8
Copy link
Contributor Author

cho8 commented Oct 31, 2019

The above is also true for when getMime is used to check content-type
e.g. 'image/jpg would have returned as /jpg

@diana-derose
Copy link
Collaborator

@cho8 - Thanks for the details, I have approved it.

@diana-derose diana-derose merged commit 2c4de22 into intuit:develop Nov 1, 2019
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.

3 participants