-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
media-params: get rid of string/token distinction #542
Conversation
From RFC7231§3.1.1.1 A parameter value that matches the token production can be transmitted either as a token or within a quoted-string. The quoted and unquoted values are equivalent. For example, the following examples are all equivalent, but the first is preferred for consistency: text/html;charset=utf-8 text/html;charset=UTF-8 Text/HTML;Charset="utf-8" text/html; charset="utf-8" The parser now simply returns a string and serializes to a token or a quoted string depending on whether the parameter includes characters that require escaping.
@dsheets this is your territory I believe |
Unfortunately, I believe that Beyond that, I think Postel's Law is applicable and we should be ready to accept (or allow users to accept) either a token or a quoted string for parameters that can be tokens or are specified as tokens (as RFC 7231 specifies for HTTP). On the flip side, I think the user should have control of the encoding of parameters and be able to specify that even parameters that could satisfy Finally, I'd really love to see a stand-alone library just to handle media types. @dinosaure's MrMime also contains parser/printer/types for media types and it looks like it does distinguish strings and tokens at the OCaml type level. Though I do love these specs and really want a highly focused @seliopou thanks for your interest in improving this functionality. Unfortunately, I think it's always been defective (and is still defective with your patch) and really needs to be factored out of cohttp and MrMime into a stand-alone component that is definitely correct, user friendly, and has no unnecessary dependencies or functionality. Hope this helps. |
Hmmhmm, I think it's possible to extract from MrMime the part about the RFC 822 and the RFC 2045 (and extract the So, I don't know the plan about |
See https://github.com/oklm-wsh/TypeBeat. It's not available on |
Please do make an opam release @dinosaure! |
Typebeat has been released (see https://github.com/ocaml/opam-repository/tree/master/packages/typebeat). Does this PR need refresh and inclusion? Or should we just close it? |
For my perspective, I agree that we should (according to the RFC) split the MIME type, |
I am happy wither way. We can also postpone this change if you think it would take too long and it needs to be rethought carefully. |
Close by #725 |
…lwt, cohttp-lwt-unix, cohttp-lwt-unix-ssl, cohttp-top, cohttp-async and cohttp-mirage (3.0.0) CHANGES: - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - port to conduit 3.0.0: minor breaking changes on the server API, an explicit distinction between cohttp-lwt-unix (using tls), cohttp-lwt-notls and cohttp-lwt-ssl (using lwt_ssl), and includes ssl in cohttp-async (@dinosaure mirage/ocaml-cohttp#692) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686)
…lwt, cohttp-lwt-unix, cohttp-lwt-unix-ssl, cohttp-top, cohttp-async and cohttp-mirage (3.0.0) CHANGES: - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - port to conduit 3.0.0: minor breaking changes on the server API, an explicit distinction between cohttp-lwt-unix (using tls), cohttp-lwt-notls and cohttp-lwt-ssl (using lwt_ssl), and includes ssl in cohttp-async (@dinosaure mirage/ocaml-cohttp#692) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686)
…lwt, cohttp-lwt-unix, cohttp-lwt-unix-ssl, cohttp-top, cohttp-async and cohttp-mirage (3.0.0) CHANGES: - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - port to conduit 3.0.0: minor breaking changes on the server API, an explicit distinction between cohttp-lwt-unix (using tls), cohttp-lwt-notls and cohttp-lwt-ssl (using lwt_ssl), and includes ssl in cohttp-async (@dinosaure mirage/ocaml-cohttp#692) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686)
…lwt, cohttp-lwt-unix, cohttp-lwt-unix-ssl, cohttp-top, cohttp-async and cohttp-mirage (3.0.0) CHANGES: - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - port to conduit 3.0.0: minor breaking changes on the server API, an explicit distinction between cohttp-lwt-unix (using tls), cohttp-lwt-notls and cohttp-lwt-ssl (using lwt_ssl), and includes ssl in cohttp-async (@dinosaure mirage/ocaml-cohttp#692) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686)
…ohttp-top, cohttp-async and cohttp-mirage (4.0.0) CHANGES: - cohttp.response: fix malformed status header for custom status codes (@mseri @aalekseyev mirage/ocaml-cohttp#752) - Remove dependency to base (@samoht mirage/ocaml-cohttp#745) - fix opam files and dependencies - add GitHub Actions workflow (@smorimoto mirage/ocaml-cohttp#739) - lwt_jsoo: Forward exceptions to caller when response is null (@mefyl mirage/ocaml-cohttp#738) - Remove wrapped false (@rgrinberg mirage/ocaml-cohttp#734) - Use implicit executable dependency for generate.exe (@TheLortex mirage/ocaml-cohttp#735) - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686) - [reverted] breaking changes to client and server API to use conduit 3.0.0 (@dinosaure mirage/ocaml-cohttp#692). However, as the design discussion did not reach consensus, these changes were reverted to preserve better compatibility with existing cohttp users. (mirage/ocaml-cohttp#741, @samoht)
…ohttp-top, cohttp-async and cohttp-mirage (4.0.0) CHANGES: - cohttp.response: fix malformed status header for custom status codes (@mseri @aalekseyev mirage/ocaml-cohttp#752) - Remove dependency to base (@samoht mirage/ocaml-cohttp#745) - fix opam files and dependencies - add GitHub Actions workflow (@smorimoto mirage/ocaml-cohttp#739) - lwt_jsoo: Forward exceptions to caller when response is null (@mefyl mirage/ocaml-cohttp#738) - Remove wrapped false (@rgrinberg mirage/ocaml-cohttp#734) - Use implicit executable dependency for generate.exe (@TheLortex mirage/ocaml-cohttp#735) - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686) - [reverted] breaking changes to client and server API to use conduit 3.0.0 (@dinosaure mirage/ocaml-cohttp#692). However, as the design discussion did not reach consensus, these changes were reverted to preserve better compatibility with existing cohttp users. (mirage/ocaml-cohttp#741, @samoht)
From RFC7231§3.1.1.1:
The parser now simply returns a string and serializes to a token or a quoted string depending on whether the parameter includes characters that require escaping.