contribsys / faktory

Language-agnostic persistent background job server
https://contribsys.com/faktory/
Other
5.78k stars 230 forks source link

Job arguments with &, <, and > are escaped when unmarshaling #440

Closed arjun810 closed 1 year ago

arjun810 commented 1 year ago

I'm encoding a job with urls as arguments (though the problem can be reproduced without URLs). Here's an example for the ruby client:

c.push({ "jobtype" => "foo", "queue" => "critical", "retry" => 10, "args" => [1, 2, "Some&String"], }) In the faktory web UI (and when pulling a job), you'll then see: "Some\u0026String" for the args.

Looks like this is the root cause: https://github.com/golang/go/issues/28453

It'd be nice to not need to alter our clients to undo this.

mperham commented 1 year ago

I don't think I'd be able to change this without a major version bump.

arjun810 commented 1 year ago

Makes sense. It looks like the issue was actually just in the dashboard. It seems that both the ruby and python clients handle this properly. I had run into an error, and copied and pasted arguments from the dashboard to debug, and thought that this was the root cause, though it wasn't.

Perhaps a first step would be to just unescape them in the dashboard?

mperham commented 1 year ago

The argument display uses json.Marshal, which escapes HTML entities. That's ok except that ego rendering also escapes HTML entities, leading to double encoding on the web page. We need to switch that json.Marshal call to instead use a custom json.Encoder with SetEscapeHTML(false).

https://github.com/contribsys/faktory/blob/fd1abcae22fbaa3d1b535e820d295b751f59cf82/webui/helpers.go#L429C1-L430C1

If you can send a PR with that fix, I'd be happy to merge. Otherwise I'll get to this next month as I'm on vacation right now.