1483670 - figure out unicode story for firefox-facing endpoints

Status: RESOLVED FIXED

(Release Engineering Graveyard :: Applications: Balrog (backend), enhancement, P1)

Description

[[e-mail] (:bhearsum)]

We recently tried to disallow unicode in most places of Balrog. One of those places was "channel" on the public endpoints, which caused a bunch of tracebacks when it was pushed. Oddly, we have a test that seems like it should've broken when unicode was disallowed (https://github.com/mozilla/balrog/blob/master/auslib/web/public/swagger/api.yml#L280), but it didn't.

We should figure out whether or not we actually should accept unicode on the public endpoints, and write some tests (that actually work!) to verify our expectations.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

The error catching code in https://github.com/mozilla/balrog/commit/9a408f437a102827ae420b680ff2f1bc7eb7a889 probably needs a tweak too, as the exceptions weren't being turned into a 400.

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

After 24 hours, Sentry says we had about 1.5k requests from 570 'users' with unicode that caused an exception. So definitely worth fixing bug not setting anything on fire.

Comment 3

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] (UTC+12) from comment #1)
> The error catching code in
> https://github.com/mozilla/balrog/commit/
> 9a408f437a102827ae420b680ff2f1bc7eb7a889 probably needs a tweak too, as the
> exceptions weren't being turned into a 400.

I think what's happened with this is that the error handler (https://github.com/mozilla/balrog/commit/9a408f437a102827ae420b680ff2f1bc7eb7a889#diff-ac81d0bfcf3f687c74dc663d02e13f7dR67) is only for the admin app, so it doesn't come into play for all the recent exceptions on the public side.

Comment 4

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/balrog/pull/755

Comment 5

[[github robot]]

Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/5467cce61099639c4f9f26f693c2c17d72700531
bug 1483670: accept unicode from update endpoints (#755)

* Poorly convert unicode to string update endpoints.

* Add a test to verify that unicode doesn't result in errors.

* Add unicode error handler on public app.

* Fix import error.

* Don't do unnecessary encoding on python3

* Fix error messages and add a bunch of comments.

* Remove debugging prints

Comment 6

[Nick Thomas [:nthomas] (UTC+12)]

Fixed deployed today, thanks Ben.

Comment 7

[Nick Thomas [:nthomas] (UTC+12)]

Did everything you wanted to get done land in that push ?

Comment 8

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] (UTC+12) from comment #7)
> Did everything you wanted to get done land in that push ?

Yup, I think this covers everything. The story ended up being "accept unicode in all fields that can contain arbitrary content".

In some distant future that story may become "support unicode at every layer of Balrog" (which would let us better deal with things such as certain partner repacks adding unicode characters to the channel) -- but that may never happen either.