mirage / ocaml-uri

RFC3986 URI parsing library for OCaml
Other
97 stars 57 forks source link

Custom percent-encoders: don't mutate base encoder #156

Closed aantron closed 3 years ago

aantron commented 3 years ago

At the moment, `Custom mutates the safe character array of its underlying component kind:

https://github.com/mirage/ocaml-uri/blob/b4a8375d9352d29ff495d35fc309609fad74631a/lib/uri.ml#L166-L176

This was added in #147. I'm not sure if this is intentional or not (cc @orbitz). I am currently abusing `Custom to build IRI-like percent encoders from existing component kinds (as a workaround for lack of #34). I don't want to modify the underlying encoders, though — I'd like both IRI and ASCII-only support for my own code, and, in addition, I fear about violating the assumptions of other code linked into the program.

orbitz commented 3 years ago

Whoops! No, this is unintentional!

aantron commented 3 years ago

Any word on a merge of this PR and release? This is needed for Dream (see https://github.com/aantron/dream/issues/50). Of course, it can be worked around by not using ocaml-uri, or using it less, but I would prefer not to add code that exists in ocaml-uri to Dream as well.

avsm commented 3 years ago

This is obviously safe and very important to release. I'll merge and cut a release now.

aantron commented 3 years ago

Thanks @orbitz and @avsm!