/synapse

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

More intelligent Content-Type parsing #1660

Merged
merged 1 commit into from Nov 30, 2016

Conversation

Reviewers
No reviews
Assignees

@NegativeMjark NegativeMjark

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@richvdh @NegativeMjark @Kegsay
@richvdh
Member

richvdh commented Nov 30, 2016

Content-Type is allowed to contain options (; charset=utf-8, for
instance). We should allow that.

@richvdh richvdh assigned NegativeMjark Nov 30, 2016

@richvdh
More intelligent Content-Type parsing 
Content-Type is allowed to contain options (`; charset=utf-8`, for
instance). We should allow that.

Some checks were not successful

1 errored, 1 failing, and 8 successful checks
@matrixbot
Sytest Dendron (Commit) — Build #1182 origin/rav/better_content_type_validation failed in 10 min
Details
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Flake8 + Packaging (Commit) — Build #2117 origin/rav/better_content_type_validation succeeded in 47 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #2019 origin/rav/better_content_type_validation succeeded in 7 min 54 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #2061 origin/rav/better_content_type_validation succeeded in 9 min 50 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #2145 origin/rav/better_content_type_validation succeeded in 2 min 32 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
b5b3a7e
@NegativeMjark
Contributor

NegativeMjark commented Nov 30, 2016

I don't think the application/json mime-type has any parameters? But I guess people are silly enough to put parameters there, otherwise you probably wouldn't be making this PR. https://tools.ietf.org/html/rfc7159#section-11

LGTM if it works.

@richvdh richvdh referenced this pull request Nov 30, 2016

Closed

matrixfederationclient: Fix Content-Type parsing #1661

@richvdh richvdh merged commit 8379a74 into develop Nov 30, 2016
8 of 10 checks passed

8 of 10 checks passed

Sytest Dendron (Commit) Build #1182 origin/rav/better_content_type_validation failed in 10 min
Details
Sytest Dendron (Merged PR) Build finished.
Details
Flake8 + Packaging (Commit) Build #2117 origin/rav/better_content_type_validation succeeded in 47 sec
Details
Flake8 + Packaging (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #2019 origin/rav/better_content_type_validation succeeded in 7 min 54 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #2061 origin/rav/better_content_type_validation succeeded in 9 min 50 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
Unit Tests (Commit) Build #2145 origin/rav/better_content_type_validation succeeded in 2 min 32 sec
Details
Unit Tests (Merged PR) Build finished.
Details

@richvdh richvdh deleted the rav/better_content_type_validation branch Nov 30, 2016

@Kegsay
Contributor

Kegsay commented Dec 1, 2016

Though if you want to nitpick, the RFC does say:

Note: No "charset" parameter is defined for this registration.
Adding one really has no effect on compliant recipients.

In this case adding one does have an effect on the recipient :D
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment