Code review comment for lp:~wallyworld/lazr.restful/propogate-notifications

On Fri, Mar 25, 2011 at 9:25 AM, Ian Booth <email address hidden> wrote:
>> 1. Custom HTTP headers need to start with X-, and by convention our custom
>> HTTP headers start with X-Lazr-. So XHR-Notifications should be X-Lazr-XHR-
>> Notifications. But there's nothing XHR-specific about these notifications.
>> That's how you're using them now, but other clients could use them for other
>> purposes. So I recommend just X-Lazr-Notification.
>>
>
> Done
>
>> 2. The more complicated the value of the HTTP header, the less comfortable I
>> feel. My ideal is for HTTP header values to be plain text. If client support
>> allows it, I would prefer to see each notification sent in a separate copy of
>> the header:
>>
>> X-Lazr-Notification: ['DEBUG', 'Notification 1']
>> X-Lazr-Notification: ['WARN', 'Notification 2']
>>
> <snip>
>
> I may be wrong - RFC2616 says
> "Multiple message-header fields with the same field-name MAY be
> present in a message if and only if the entire field-value for that
> header field is defined as a comma-separated list [i.e., #(values)].
> It MUST be possible to combine the multiple header fields into one
> "field-name: field-value" pair, without changing the semantics of the
> message, by appending each subsequent field-value to the first, each
> separated by a comma"

Yep. Unfortunately multiple headers aren't really meant to be "true"
sequences just shorthand for a comma-separated list. We could escape
any commas in the headers but we're starting to get hacky.

> Since the notification text values may be escaped html and totally
> free form text, it's not possible I think to do what is suggested
> above. The only way I can see at this point is to encode the
> notification text values as a json string and send as one header
> value.

Do we really want to allow arbitrary HTML? For example, what if I have
a GUI client and want to display these messages? I would have to use an
HTML renderer to be able to show them sanely. We could allow a small
subset of HTML like just anchor tags, but even that places a pretty big
burden on non-browser clients.

I propose that we start with a type (DEBUG, WARNING, etc.) and a
plain-text string. That may mean that we have to audit/rework the
existing notifications to make them compatible.
--
Benji York