🐛 [Bug]: ctx.Accepts("text", "json") with headerAccept, "text/*, application/json" fails on macOS

[sixcolors]

Bug Description

ctx.Accepts("text", "json") with headerAccept, "text/*, application/json" is not the same on macOS and linux

On macOS it returns "text"

On linux it returns "json"

How to Reproduce

Run test on macOS and linux

Expected Behavior

consistent return and tests pass

Fiber Version

v2.43.0

Code Snippet (optional)

func Test_Ctx_Accepts(t *testing.T) {
	t.Parallel()
	app := New()
	c := app.AcquireCtx(&fasthttp.RequestCtx{})
	defer app.ReleaseCtx(c)

	c.Request().Header.Set(HeaderAccept, "text/*, application/json")
	utils.AssertEqual(t, "json", c.Accepts("json", "text"))
}

// called with above params DEBUG comments added
func (c *Ctx) Accepts(offers ...string) string {
	if len(offers) == 0 {
		return ""
	}
	header := c.Get(HeaderAccept)
	if header == "" {
		return offers[0]
	}

	spec, commaPos := "", 0
	for len(header) > 0 && commaPos != -1 {
		commaPos = strings.IndexByte(header, ',')
		if commaPos != -1 {
			spec = utils.Trim(header[:commaPos], ' ')
		} else {
			spec = utils.TrimLeft(header, ' ')
		}
		if factorSign := strings.IndexByte(spec, ';'); factorSign != -1 {
			spec = spec[:factorSign]
		}

		var mimetype string
		for _, offer := range offers { // DEBUG first loop "text"
			if len(offer) == 0 {
				continue
				// Accept: */*
			} else if spec == "*/*" {
				return offer
			}

			if strings.IndexByte(offer, '/') != -1 {
				mimetype = offer // MIME type
			} else {
				mimetype = utils.GetMIME(offer) // DEBUG on linux: "application/octet-stream", on macOS: "text/plain; charset=utf-8"
			}

			if spec == mimetype {
				// Accept: <MIME_type>/<MIME_subtype>
				return offer
			}

			s := strings.IndexByte(mimetype, '/')
			// Accept: <MIME_type>/*
			if strings.HasPrefix(spec, mimetype[:s]) && (spec[s:] == "/*" || mimetype[s:] == "/*") { // DEBUG first loop mimeType = "text/*"
				return offer // DEBUG on macOS text spec matches the string before / (text), on linux text does not match string before / (application)
			}
		}
		if commaPos != -1 {
			header = header[commaPos+1:]
		}
	}

	return ""
}

Further context

Caused by: utils.GetMIME("text"), which calls mime.TypeByExtension("." + extension) on macOS returns "text/plain; charset=utf-8" while on linux it returns ""

Checklist:

• I agree to follow Fiber's Code of Conduct.
• I have checked for existing issues that describe my problem prior to opening this one.
• I understand that improperly formatted bug reports may be closed without explanation.

[sixcolors]

If we add "text" to http.go mime extensions

// - Add "text" as an alias for "text/plain" as recommended per https://datatracker.ietf.org/doc/html/rfc2046#section-4.1var mimeExtensions = map[string]string{
var mimeExtensions = map[string]string{
...
"text":    "text/plain",
"txt":     "text/plain",
...
}

Then behaviour becomes consistent across platforms.

This still leaves the question of if subtypes take priority in the same way that types do in the mime headers. https://docs.gofiber.io/api/ctx#accepts indicates such, as does test assertion in ctx_test.go to line 52.

If so the code does not do that and only by coincidence of mime.TypeByExtension(".text") returning "" does GetMIME default to returning MIMEOctetStream which does not match "text" in as the first spec and returning early from the for a loop.

[sixcolors]

The section of RFC 2046 that defines the "text" media type and its subtypes is Section 4.1. In this section, the RFC states:

The top-level media type "text" has a number of subtype values which
indicate human-readable text in one form or another. The primary
subtype, "plain", indicates plain (unformatted) text.

...

Other subtypes of "text" usefully include types that represent
formatted text containing controlled white space (e.g., "text/tab-
separated-values") and text representing markup languages (e.g.,
"text/html").

...

It is important to note that "text" cannot be assumed to be
anything other than a structured byte sequence whose precise
structure is not generally known to the recipient.

...

The recommended practice for the Internet is to label human-
readable content types as "text/plain" and most machine-readable
formats as "application/octet-stream".

This passage implies that "text" is a top-level media type that encompasses various subtypes, including "text/plain". Later in the same section, the RFC notes that "text/plain" is the recommended subtype for human-readable content types, suggesting that "text" can be used as an alias for "text/plain" in these contexts.

[ReneWerner87]

Its related to #2340

[sixcolors]

Its related to #2340

I think adding text to the mime map would be an acceptable solution to normalize the behavior.

But what about the specific scenario: if text is a mime type matching text/ in the inner loop should that return text? Or should it check for type or subtype match and return json?

If so the Accepts code needs to be updated.

Based on the test my thinking is the loops are reversed. We should check the offers first then see if they match one of the headerAccept mime types. Perhaps using a [acceptHeader] type/subtype map.

That also makes me wonder about media type parameters (if we need to match those too).

Thoughts?

[sixcolors]

And what about the q value? https://developer.mozilla.org/en-US/docs/Glossary/Quality_values

[ReneWerner87]

@derkan can you help here

[derkan]

@sixcolors, what is the go version?

I've tested with go1.19.7 and by adding a new test line to ctx_test.go like this:

...
c.Request().Header.Set(HeaderAccept, "text/*, application/json")
...
utils.AssertEqual(t, "json", c.Accepts("text", "json"))

Unfortunately I got same result: "json" on macOS for both go versions(and fiber master@latest). Am I missing something or this problem is because of go version?

[sixcolors]

@sixcolors, what is the go version?

I've tested with go1.19.7 and by adding a new test line to ctx_test.go like this:

...
c.Request().Header.Set(HeaderAccept, "text/*, application/json")
...
utils.AssertEqual(t, "json", c.Accepts("text", "json"))

Unfortunately I got same result: "json" on macOS for both go versions(and fiber master@latest). Am I missing something or this problem is because of go version?

1.20.2

[sixcolors]

@sixcolors, what is the go version?
I've tested with go1.19.7 and by adding a new test line to ctx_test.go like this:

...
c.Request().Header.Set(HeaderAccept, "text/*, application/json")
...
utils.AssertEqual(t, "json", c.Accepts("text", "json"))

Unfortunately I got same result: "json" on macOS for both go versions(and fiber master@latest). Am I missing something or this problem is because of go version?

1.20.2

However it probably has more to do with macOS install than go version looking at https://cs.opensource.google/go/go/+/refs/tags/go1.20.2:src/mime/type.go;l=102

I’ll check the files when I’m back on a computer (on phone right now)

[derkan]

I've also tested with following line added to ctx_test.go on macOS and go1.20.2, no problem.

c.Request().Header.Set(HeaderAccept, "text/*, application/json")
utils.AssertEqual(t, "json", c.Accepts("text", "json"))

[sixcolors]

I'm seeing .text registered with text edit. So that should be on basically every macOS install...

mdls -name kMDItemContentTypeTree example.text
kMDItemContentTypeTree = (
"public.plain-text",
"public.text",
"public.data",
"public.item",
"public.content"
)

duti -x text
TextEdit.app
/System/Applications/TextEdit.app
com.apple.TextEdit

let me write a sample app.

[sixcolors]

@derkan made a sample and test pass using CI GitHub action but fails locally. Checking a fresh install of macOS to make sure.

https://github.com/sixcolors/textmime/
https://github.com/sixcolors/textmime/actions/runs/4524905041

~/textmime (main) » go test -v ./... sixcolors@Jasons-MacBook-Pro
=== RUN TestAccepts
main_test.go:13: mimetype is text/plain; charset=utf-8, want application/json
--- FAIL: TestAccepts (0.00s)
FAIL
FAIL github.com/sixcolors/textmime 0.596s
FAIL

[sixcolors]

@derkan yes it fails on multiple local macOS versions including a fresh install of the latest macOS

Please see the test project.

[sixcolors]

@ReneWerner87 and @derkan

The other part is this:

...
c.Request().Header.Set(HeaderAccept, "text/*, application/json")
...
utils.AssertEqual(t, "json", c.Accepts("json", "txt"))
utils.AssertEqual(t, "json", c.Accepts("json", "text/plain"))

Those should still pass, right? If so then the logic in Accepts is wrong.

[ReneWerner87]

I think the accepts method works wrong

instead of going through the arguments in the loop and comparing them one by one with the header values

the header values are split and compared with the passed arguments one after the other