gleam-wisp / wisp

🧚 A practical web framework for Gleam
https://gleam-wisp.github.io/wisp/
Apache License 2.0
915 stars 41 forks source link

Is it possible to support non US ASCII header values #45

Closed finalclass closed 1 year ago

finalclass commented 1 year ago

Hi, I'm sending a custom header value using wisp. I'm doing it like this:

image

The header has some polish letters which normally are well displayed in any device supporting utf8. However when I receive the request in the browser, the encoding is invalid:

image

Is there a way to send a BitString instead?

lpil commented 1 year ago

Hello! From my reading of RFC 7230 header values can only contain US ASCII. I tested with some Python and JavaScript servers and frameworks and they would all crash when given a unicode header like that.

What are you proposing that Wisp or Mist does instead here? Thank you

P.S. Hey you're using HTMX with Wisp! Really cool!!

finalclass commented 1 year ago

Ahh, sorry for bothering you in this case. I'll have to figure out something else for notifications. Maybe I'll encode the string before sending it. We'll see.

Yes, Gleam with Wisp is amazing and it pairs great with htmx. Now that I've started working with Gleam, I just don't want to go back to any other language. It's truly amazing.

lpil commented 1 year ago

Thank you so much! I hope we can keep making it better and better 💜

I do agree with you, it would be very handy to be able to use unicode here, for htmx and others. I wonder if there's something clever that other frameworks do here.

finalclass commented 1 year ago

I checked expressjs and it produces the error:

TypeError [ERR_INVALID_CHAR]: Invalid character in header content ["hx-trigger"]
    at ServerResponse.setHeader (node:_http_outgoing:647:3)

I also took a look how it's done in PHP running on apache2 and PHP simply does nothing (just returns ascii with some garbage).

So IMO wisp is correct here, it behaves like PHP.


For anyone having this problem this is how I solved it:

  1. On the server side I encode the string like this:
@external(erlang, "uri_string", "quote")
fn erlang_encode_uri_component(s: Charlist) -> Charlist

fn escape_uri(s: String) -> String {
  s
  |> charlist.from_string()
  |> erlang_encode_uri_component()
  |> charlist.to_string()
}

pub fn flash_message(res: Response, level: String, message: String) -> Response {
  let message = escape_uri(message)

  res
  |> wisp.set_header(
    "hx-trigger",
    "{\"alert\": {\"message\": \"" <> message <> "\", \"level\": \"" <> level <> "\"}}",
  )
}

This is how I use it:

          wisp.not_found()
          |> flash_message(
            "error",
            "Operacja nieudana, nie można usunąć ostatniego administratora",
          )

In the browser I then run the messages through: decoceURIComponent like so:

          UlA(
            [
              a.Class("messages"),
              a.XData("{messages: []}"),
              a.XOn(
                "alert.window",
                "messages.push($event.detail); setTimeout(() => messages.shift(), 10000)",
              ),
            ],
            z.TemplateA(
              [a.XFor("message in messages")],
              z.LiA(
                [
                  a.XTransition,
                  a.XText("decodeURIComponent(message?.message)"),
                  a.XBind("class", "message?.level"),
                ],
                z.Empty,
              ),
            ),
          ),

(I'm using alpinejs and my custom html builder library here but you should get a point what it's doing. I will release the lib when it's ready)

lpil commented 1 year ago

Both approaches are correct, but I think the crash approach is somewhat nicer as it doesn't cryptically mangle headers, instead it tells the programmer that they've made a mistake. Ultimately it's up to Mist to handle these headers however it wants though, it's not something that Wisp has control over.

finalclass commented 1 year ago

Yes, both are correct so Wisp approach is also correct. Express approach is indeed a little bit nicer however it adds some overhead to the server. Since it's a Mist area then I guess we could close this one?

lpil commented 1 year ago

OK! Thank you.