ampproject / ampproject/amppackager

Fix Accept parsing

Open
#240 1 comment 0 reactions 0 assignees View on GitHub
fixit
Dominant language
Go
Stars
141
Forks
46
PR merge metrics
No merged PRs in 30d

Description

There is a bug in the comma-splitting logic for Accept (not fixed in #196). It doesn't ignore commas inside quoted strings. A correct splitter would need to identify `,`, `"`, and `\"`. See [Spring's tokenizer](https://www.google.com/url?q=https://github.com/spring-projects/spring-framework/blob/7bbd4c681fd009898a62cac6e11eae77039a4119/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java%23L274&sa=D&source=hangouts&ust=1548433024564000&usg=AFQjCNFkqlocOoRDdAL9J8mObQTnU_fTBw) for example.

This bug is mentioned in `accept_test.go`, but downplayed as rare. However, it is likely to occur in the future when Chrome advertises support for `v="b3,b4"`.

Contributor guide

Open the contributing guide

Research direction

Start with accept_test.go and reproduce the Accept header case containing a quoted comma, such as v="b3,b4". Compare the comma-splitting behavior with Spring's tokenizer reference, then verify that commas inside quoted strings and escaped quotes are handled correctly. Done means the regression case passes without breaking ordinary Accept parsing.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend-api-design
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.