Add support for CORS in WebMessageHandler

[lkraider]

Allow easy support for Cross-Origin Resource Sharing requests by simply returning a list of hosts on the cors_allow_origin method.

[gone]

Dude awesome job!

I really like the idea of using prepare to determine if the resource is CORS-able or not - but can you split that out into a separate decorator? My understanding is that prepare is really meant for subclasses to overwrite - I don't want to depend on functionality there. Also making it into a decorator allows people to explicitly mark which resources are for cors and which ones are not.

Can you make origin, headers, methods, exposed headers, and allowed credentials properties, rather than methods? Keeping those static as much as possible should make things faster.

What do you think about turning the part that handles credentials/allow-origin = * into a function so it's not duplicated? I could go either way on that one.

Thoughts?

[lkraider]

Thanks for reviewing it!

I was already pending towards defining it as a decorator, makes sense.

Turning those methods into simple instance attributes is better. I think I'll override __init__ for that then, as initialize is called on clear_payload and it's unnecessary to reset those values there. Although subclasses will probably redefine them there.

I thought about turning that snippet into a function, but since on runtime it's basically a condition check and an assignment, I thought a function call would be more overhead than repeating a little code.

[gone]

that's probably true that a function is more overhead.

I think since they're static they should be class properties, rather than using init. If subclasses need dynamic attributes, they can use @property

I think that will be faster/less overhead, and semantically I think it's better - resources generally don't have different CORS rules depending on the request.

On Tue, Feb 19, 2013 at 4:38 PM, Paul Eipper notifications@github.comwrote:

Thanks for reviewing it!

I was already pending towards defining it as a decorator, makes sense.

Turning those methods into simple instance attributes is better. I think I'll override init for that then, as initialize is called on clear_payload and it's unnecessary to reset those values there. Although subclasses will probably redefine them there.

I thought about turning that snippet into a function, but since on runtime it's basically a condition check and an assignment, I thought a function call would be more overhead than repeating a little code.

— Reply to this email directly or view it on GitHubhttps://github.com/j2labs/brubeck/pull/106#issuecomment-13801306.

[gone]

Also this is so, so slick. I have a bunch of crappy patched @cors_support resources in an app that just set allowed headers -I'd love to replace those with something better designed.

On Tue, Feb 19, 2013 at 4:50 PM, Ben Beecher benbeecher@gmail.com wrote:

that's probably true that a function is more overhead.

I think since they're static they should be class properties, rather than using init. If subclasses need dynamic attributes, they can use @property

I think that will be faster/less overhead, and semantically I think it's better - resources generally don't have different CORS rules depending on the request.

On Tue, Feb 19, 2013 at 4:38 PM, Paul Eipper notifications@github.comwrote:

Thanks for reviewing it!

I was already pending towards defining it as a decorator, makes sense.

Turning those methods into simple instance attributes is better. I think I'll override init for that then, as initialize is called on clear_payload and it's unnecessary to reset those values there. Although subclasses will probably redefine them there.

I thought about turning that snippet into a function, but since on runtime it's basically a condition check and an assignment, I thought a function call would be more overhead than repeating a little code.

— Reply to this email directly or view it on GitHubhttps://github.com/j2labs/brubeck/pull/106#issuecomment-13801306.

[lkraider]

I believe I addressed your points with these commits, although I didn't test the code thoroughly after the refactoring.