Skip to content

fix: the MIME type of an SVG file should be image/svg+xml#286

Merged
System-Glitch merged 7 commits into
go-goyave:masterfrom
fnoopv:fix_svg_mime_detect
Mar 9, 2026
Merged

fix: the MIME type of an SVG file should be image/svg+xml#286
System-Glitch merged 7 commits into
go-goyave:masterfrom
fnoopv:fix_svg_mime_detect

Conversation

@fnoopv

@fnoopv fnoopv commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

References

Issue(s): #xxx
Discussion: #yyy

Description

The MIME type of an SVG file should be image/svg+xml, otherwise, the front-end cannot render the image based on the MIME type.

Possible drawbacks

@coveralls

coveralls commented Mar 2, 2026

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 22859273349

Details

  • 27 of 37 (72.97%) changed or added relevant lines in 2 files are covered.
  • 39 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 96.903%

Changes Missing Coverage Covered Lines Changed/Added Lines %
util/fsutil/fsutil.go 23 33 69.7%
Files with Coverage Reduction New Missed Lines %
util/fsutil/fsutil.go 39 67.84%
Totals Coverage Status
Change from base Build 21984554444: -0.02%
Covered Lines: 6696
Relevant Lines: 6910

💛 - Coveralls

@System-Glitch System-Glitch self-requested a review March 2, 2026 09:22
@System-Glitch System-Glitch self-assigned this Mar 2, 2026
@System-Glitch System-Glitch added the bug Something isn't working label Mar 2, 2026
Comment thread util/fsutil/file.go Outdated
Comment thread util/fsutil/fsutil.go Outdated
Comment thread util/fsutil/fsutil.go Outdated
if strings.HasSuffix(fileName, ext) {
tmp := t
if i := strings.Index(contentType, ";"); i != -1 {
if i := strings.Index(contentType, ";"); i != -1 && t != "image/svg+xml" {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need this condition?

@fnoopv fnoopv Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

image/svg+xml usually does not need to be concatenated with charset=utf-8. If it exists, remove it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see. This is because http.DetectContentType returns text/xml; charset=utf-8 and we want just image/svg+xml.

This is not necessary anymore since you added the SVG sniffing. Can you remove this change please?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

Comment thread util/fsutil/fsutil.go Outdated
if strings.HasSuffix(fileName, ext) {
tmp := t
if i := strings.Index(contentType, ";"); i != -1 {
if i := strings.Index(contentType, ";"); i != -1 && t != "image/svg+xml" {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see. This is because http.DetectContentType returns text/xml; charset=utf-8 and we want just image/svg+xml.

This is not necessary anymore since you added the SVG sniffing. Can you remove this change please?

Comment thread util/fsutil/fsutil.go
return strings.HasPrefix(content, "<svg")
}

func detectContentTypeByExtension(fileName, contentType string) string {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add an early return here if fileName is empty to avoid extra processing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

Comment thread util/fsutil/fsutil.go Outdated
Comment thread util/fsutil/fsutil.go Outdated
Comment thread util/fsutil/fsutil.go
Comment thread util/fsutil/fsutil.go
Comment thread util/fsutil/fsutil_test.go
fnoopv and others added 3 commits March 9, 2026 16:42
Co-authored-by: SystemGlitch <jeremy.la@outlook.fr>
Co-authored-by: SystemGlitch <jeremy.la@outlook.fr>
@System-Glitch

Copy link
Copy Markdown
Member

LGTM 👍🏻 I'll make a couple minor changes before merging.

@System-Glitch System-Glitch merged commit 512d57b into go-goyave:master Mar 9, 2026
5 checks passed
@System-Glitch

Copy link
Copy Markdown
Member

Merged 🎉 Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants