Development

IRCLogs20110310-2

You must first sign up to be able to contribute.

Version 3 (modified by pgodel, 7 years ago)
--

IRC logs of Part 2

[1:40 PM] <kriswallsmith> so, johanness needs answers for subrequest security and eager response headers
[1:40 PM] <kriswallsmith> shall we discuss?
[1:40 PM] <naderman> johanness: around?
[1:42 PM] <naderman> for eager response headers I think the only question left is whether there should be a generic core.response listener which takes a response header bag from the request and turns it into headers or whether there should be individual listeners turning request attributes into headers
[1:42 PM] <kriswallsmith> still doesn't seem right to me
[1:42 PM] <naderman> the latter solution avoids setting response headers before you have a response and at the same time it will probably be more code
[1:43 PM] <kriswallsmith> the purpose of having it eager is so a controller can make changes
[1:43 PM] <kriswallsmith> otherwise core.response would be sufficient
[1:43 PM] <naderman> there is another reason
[1:43 PM] <naderman> if you have code that needs to run in core.request to provide your controller with information, and based on that same information you also want to set headers in core.response
[1:44 PM] <naderman> then to avoid code duplication you need to either set the response header in core.request or add the header in core.response based on that piece of information you added to the request for the controller in core.request
[1:44 PM] <kriswallsmith> you could stash on the listener
[1:45 PM] <kriswallsmith> but then the controller couldn't make changes
[1:45 PM] <naderman> yes
[1:45 PM] <kriswallsmith> so the only reason is so the controller can make changes
[1:45 PM] <johanness> kriswallsmith, what if your listener is not responsible for setting the headers?
[1:45 PM] <naderman> stashing on the listener however makes it difficult with subrequests
[1:45 PM] <kriswallsmith> naderman: the listener should be request scope
[1:45 PM] <naderman> kriswallsmith: right that works too
[1:46 PM] <naderman> kriswallsmith: the problem johanness has with this is that he does not want to have a separate listener to set the headers
[1:46 PM] <Stof> not for security listeners unless the whole security is request scope
[1:46 PM] <kriswallsmith> johanness: in that case it has nothing to do with this discussion, right?
[1:46 PM] <naderman> which I don't really understand
[1:46 PM] <johanness> kriswallsmith, no the listener is delegating to another service
[1:46 PM] <naderman> johanness: I don't understand that?
[1:46 PM] <kriswallsmith> and the other service sets headers?
[1:47 PM] <naderman> you mean the service decides what headers need to be set and what needs to be provided to the controller?
[1:47 PM] <johanness> kriswallsmith, yes exactly
[1:47 PM] <naderman> but why wouldn't the service just stash the info internally until you ask it for the headers?
[1:48 PM] <kriswallsmith> the delegated service could listen to core.response
[1:48 PM] <johanness> i think having a lazy response header bag on the request is not gonna hurt, and seems like a good compromise to me
[1:48 PM] <naderman> so you ask the service something on core.request which makes it respond and generate info for the header
[1:48 PM] <naderman> and on core.response you ask the service to add its headers to the response
[1:48 PM] <kriswallsmith> my concern is that we put it there and the only time it's used in for security
[1:48 PM] <kriswallsmith> *is
[1:49 PM] <naderman> a generic response header bag also has little use for overwriting by a controller because it'd have to parse the headers to figure out what to modify
[1:50 PM] <johanness> kriswallsmith, but most people won't care anyway, it's not like it's affecting their code immensely
[1:50 PM] <naderman> I think I agree with kris on this, either store info in the listener / in the service the listener delegates to, or if the controller needs to overwrite it, put it into a request attribute
[1:50 PM] <kriswallsmith> we'll still have to maintain it
[1:50 PM] <naderman> that way we stay true to the idea of creating a response only once
[1:50 PM] <kriswallsmith> it adds complexity
[1:51 PM] <johanness> complexity?
[1:51 PM] <naderman> kriswallsmith: I think it's simpler actually, just not as "clean"
[1:51 PM] <kriswallsmith> yeah, merging in those headeres
[1:51 PM] <johanness> kriswallsmith, it will only be merged if there is a header bag
[1:51 PM] <naderman> hmm right it's intransparent in a sense so I guess you could say it's complex
[1:51 PM] <johanness> since most people won't use it, it will not be there
[1:51 PM] <naderman> johanness: yes but what do you do if there is a Content-Type header in the header bag
[1:52 PM] <naderman> do you overwrite the one in the response already?
[1:52 PM] <naderman> do you keep the one already in the response?
[1:52 PM] <johanness> but at least there is an easy way to set header from anywhere
[1:52 PM] <naderman> you can't afterall just send both
[1:52 PM] <kriswallsmith> on the otherhand, set-cookie should be allowed multiple times...
[1:52 PM] <kriswallsmith> complexity
[1:52 PM] <naderman> yes
[1:52 PM] <johanness> the response has precendence obviously
[1:53 PM] <naderman> ok so for Content-Type the response has precedence, for Set-Cookie they are both added (unless the cookie name is the same?)
[1:53 PM] <naderman> this really does sound rather complex
[1:53 PM] <johanness> naderman, we already have such kind of logic atm
[1:53 PM] <johanness> like setting a content type if none is present
[1:54 PM] <kriswallsmith> yes, but that's specific to content type
[1:54 PM] <naderman> johanness: but we don't have generic header merging code
[1:54 PM] <johanness> and to charset
[1:54 PM] <naderman> I mean this code would have to be able to consider _all_ possible headers
[1:54 PM] <naderman> and deal with them appropriately
[1:54 PM] <johanness> naderman, they are key value paris
[1:54 PM] <naderman> I mean what do you even do with X- headers?
[1:54 PM] <johanness> it's not a big deal
[1:54 PM] <kriswallsmith> johanness: it's not that simple
[1:55 PM] <kriswallsmith> the same key can be used multiple times
[1:55 PM] <naderman> johanness: they are key value pairs, where some keys may occur multiple times, but not all
[1:55 PM] <johanness> so which can occur multiple times?
[1:55 PM] <kriswallsmith> i don't think we can abstract a mechanism for this
[1:55 PM] <naderman> and you have no way of knowing which ones may or may not be added multiple times
[1:55 PM] <kriswallsmith> Set-Cookie
[1:55 PM] <naderman> johanness: any header I make up myself starting with X-
[1:55 PM] <johanness> ok cookies is special we already cover that
[1:55 PM] <naderman> but only if I want it to
[1:56 PM] <johanness> naderman, can you elaborate?
[1:56 PM] <kriswallsmith> i think each feature that needs this will have to implement its own explicit logic
[1:56 PM] <naderman> johanness: you can have custom headers, whether or not those are allowed to be duplicate is up to you
[1:56 PM] <kriswallsmith> which is fine, since most people won't use it
[1:57 PM] <naderman> Via is another header which can occur multiple times
[1:57 PM] <naderman> Warning too
[1:57 PM] <johanness> kriswallsmith, so you ask me to implement magic in security instead of having it in the kernel?
[1:57 PM] <kriswallsmith> yes
[1:57 PM] <naderman> Link too
[1:57 PM] <naderman> etc.
[1:57 PM] <kriswallsmith> behind the interface of the security component
[1:57 PM] <johanness> i don't see how headers are the domain of security
[1:57 PM] <naderman> johanness: the security thing is not magic, it is limited to Set-Cookies in one particular case
[1:57 PM] <naderman> you do not need generic header setting in security
[1:58 PM] <Seldaek> ah crap irc meeting..
[1:58 PM] <Seldaek> totally forgot
[1:58 PM] <kriswallsmith> Seldaek: this is part deux
[1:58 PM] <kriswallsmith> irc meeting reloaded
[1:58 PM] <naderman> <kriswallsmith> i think each feature that needs this will have to implement its own explicit logic <-- yes, best solution imho
[1:59 PM] <johanness> naderman, so we can have a early response cookies :)
[1:59 PM] <naderman> johanness: why do you keep insisting to make a generic solution for this, rather than solving the one problem at hand?
[1:59 PM] <johanness> because it's not easy to implement and the code gets quite ugly
[1:59 PM] <Stof> fabpot: here ?
[1:59 PM] <naderman> johanness: I disagree :D
[2:00 PM] <johanness> cookies should be fairly simple to implement and are the most common use case
[2:00 PM] <naderman> why don't you just set the cookies on core.response
[2:00 PM] <johanness> naderman, because of sub-requests etc.
[2:00 PM] <naderman> johanness: kriswallsmith already pointed out that the listner should have request scope
[2:00 PM] <naderman> so you don't need to worry about sub-requests
[2:01 PM] <kriswallsmith> johanness has pointed out that would add considerable overhead to security
[2:01 PM] <johanness> naderman, again that is not working for security
[2:01 PM] <naderman> well in that case use my solution?
[2:01 PM] <naderman> just stash listeners on sub-request core.request and unstash them on core.response?
[2:02 PM] <johanness> naderman, not trivial will make entire security even more complex
[2:02 PM] <naderman> you have that problem either way, even with "eager cookies"
[2:02 PM] <johanness> why?
[2:02 PM] <lsmith> johanness: tge question is in light of ESI .. how often will there really be subrequests for people that care about performance?
[2:02 PM] <kriswallsmith> you could either stash the response headers on the listener, organized by request using SplObjectStorage, or hijack a request attribute and stash the headers there
[2:02 PM] <naderman> johanness: where is the core.response listener that puts the cookies into the response?
[2:03 PM] <lsmith> so imho we should care about performance here so much
[2:03 PM] <naderman> lsmith: I do imagine they will be used a fair bit
[2:03 PM] <lsmith> the focus should be reliability and ease of maintenance
[2:03 PM] <naderman> lsmith: e.g. for the ajax-ability of requesting some sub-part?
[2:03 PM] <johanness> i mean i could implement a generic listener in the SecurityBundle which basically does that
[2:04 PM] <johanness> but i think it would better be placed in the ResponseListener
[2:04 PM] <lsmith> naderman: its easy to setup ESI and that is what people should go for .. at which point subrequests are turned into master requests
[2:04 PM] <naderman> right they will end up using AppCache anyway
[2:04 PM] <naderman> johanness: I think lukas is right, performance for sub-requests is not that important
[2:05 PM] <lsmith> meaning if we put a lot of effort into optimizing subrequests .. we are not benefiting ESI users
[2:05 PM] <johanness> another thing you should consider is how security integrates with other frameworks
[2:08 PM] <lsmith> johanness: which points to do see to consider there?
[2:09 PM] <naderman> I think other frameworks are all the more reason not to put something like a cookie stash into the framework
[2:09 PM] <naderman> because security would need to be able to deal with a framework that does not do that
[2:09 PM] <naderman> johanness: so I think that argument works against your solution ;-)
[2:09 PM] <johanness> naderman, not at all
[2:10 PM] <naderman> I mean having it inside security would work for other frameworks too
[2:10 PM] <johanness> naderman, but it is magically set on the response via stashing it in an splobjectstorage
[2:11 PM] <naderman> johanness: what?
[2:11 PM] <johanness> the contract of the interface is weakened
[2:11 PM] <naderman> I don't understand what you mean
[2:11 PM] <naderman> in which case is what stashed where?
[2:11 PM] <naderman> what solution are we talking about now?
[2:11 PM] <naderman> a cookie bag on the request in symfony2?
[2:11 PM] <naderman> or a cookie bag in the firewall?
[2:11 PM] <naderman> or no cookie bag at all?
[2:12 PM] <johanness> naderman, what solution are you talking about? :)
[2:12 PM] <naderman> johanness: the first one
[2:12 PM] <lsmith> i think at this point we have established that the remember me cookie needs to be checked at the start of the request
[2:12 PM] <lsmith> and that in the end something needs to be set on the reponse
[2:12 PM] <naderman> if you put the cookie bag into the symfony2 request class, then the security component will not work with a framework that cannot set cookies before it has a response
[2:13 PM] <lsmith> as such its clear we need to have some place to set something which will write into the reponse as soon as the reponse exists
[2:13 PM] <lsmith> agreed?
[2:13 PM] <naderman> yes
[2:13 PM] <naderman> the question is does it have to be a generic "write anything into request" or should there be a remember me cookie into response writer that is limited to this particular problem
[2:14 PM] <johanness> hm, ok we could have a general SecurityListener which only checks for one specific attribute
[2:15 PM] <johanness> that seems good, then i can still set it on the request but it would be no general set cookie feature
[2:15 PM] <naderman> yes
[2:15 PM] <naderman> that seems like a fair solution to me
[2:15 PM] <johanness> ofc if someone else uses that attribute it would be applied as well, but ok
[2:15 PM] <naderman> well make the name unique enough ;-)
[2:16 PM] <johanness> kriswallsmith, do you agree with this?
[2:16 PM] <naderman> although as per kriswallsmith, I'm not sure if a request attribute is necessarily the best place to store this
[2:16 PM] <kriswallsmith> using a request attribute?
[2:16 PM] <johanness> kriswallsmith, using a request attribute that only works for one specific cookie
[2:16 PM] <kriswallsmith> sounds good
[2:17 PM] <naderman> but it certainly solves the subrequest problem, so go for it
[2:24 PM] <beberlei> can someone point me to a good symfony introduction talk from one of the last conferences?
[2:24 PM] <beberlei> i need a starting point for my usergroup talk
[2:24 PM] <lsmith> beberlei: i used jordi last time
[2:24 PM] <lsmith> updated it last in early february IIRC
[2:24 PM] <lsmith> let me find it
[2:26 PM] <lsmith> beberlei: http://slides.seld.be/
[2:27 PM] <lsmith> specifically http://slides.seld.be/?file=2011-02-01+Symfony2+Introduction.html#1
[2:27 PM] <beberlei> do you have the raw also?
[2:27 PM] <lsmith> this is raw
[2:27 PM] <lsmith> you need slippy from github
[2:27 PM] <lsmith> its plain html
[2:27 PM] <lsmith> you can modify as you wish
[2:27 PM] <beberlei> its html
[2:27 PM] <beberlei> not plain html
[2:28 PM] <lsmith> its easy to edit .. this is the entire point of slippy
[2:28 PM] <lsmith> slippy also supports to in the end inline all css/js/images to make self contained versions
[2:28 PM] <beberlei> but i still have to write the code in <pre> and escape it myself?
[2:28 PM] <beberlei> doesnt look very easy to me
[2:29 PM] <beberlei> i use slidedown, which is a jquery plugin + markdown for previous talks
[2:29 PM] <lsmith> seems easy to me .. but thats all i can offer :)
[2:30 PM] <beberlei> thanks :D
[2:33 PM] <henrikbjorn> lsmith: you are the one taling to Mathew ? :)
[2:34 PM] <lsmith> henrikbjorn: uhm .. about what?
[2:34 PM] <henrikbjorn> zend stuff.. cant find the post where the zend component dependency tool is listed
[2:34 PM] <lsmith> i contacted him about maing zend\log less painful to include in Symfony2
[2:35 PM] <lsmith> henrikbjorn: https://github.com/weierophinney/zf-examples/tree/master/zf-utils
[2:35 PM] <henrikbjorn> YEEEESSS
[2:35 PM] <henrikbjorn> i owe you :)!!!!
[2:36 PM] <lsmith> do i get one "shut up now, no questions anymore" ? :)
[2:36 PM] <henrikbjorn> yes :)
[2:37 PM] <lsmith> juchu! :)
[2:38 PM] <johanness> ok, we still need to talk about subrequest security; atm, we do not secure sub-requests in the sense that everything in the configuration (access_control) is not applied to sub requests
[2:38 PM] <johanness> this can get especially confusing if you secure your controllers by relying on the _controller attribute
[2:39 PM] <naderman> I think security should work the same for subrequests as for regular requests
[2:39 PM] <lsmith> johanness: imho we should secure subrequests, but we should not try to get fancy with performance optimization
[2:39 PM] <naderman> so that ESI/no-ESI makes no difference from a security perspective
[2:39 PM] <johanness> lsmith, moving everything to scope request is not solving our problem
[2:39 PM] <lsmith> now one thing where i keep getting mentally confused is how to handle if inner requests then fail because the user does not have sufficient access rights
[2:40 PM] <johanness> i'm infact very unsure if that would work at all
[2:40 PM] <lsmith> its easy if the subrequest is done inside a controller
[2:40 PM] <lsmith> but less so when done inside a template
[2:40 PM] <henrikbjorn> the on-error thing for include tag ?
[2:40 PM] <johanness> the handle() method has a catch parameter
[2:41 PM] <lsmith> henrikbjorn: yeah .. i guess really thats all thats needed
[2:41 PM] <lsmith> aka .. either turn the entire request into failed .. or just ignore and return empty content
[2:41 PM] <henrikbjorn> you would still get the error in a log, and through you notification system get notified
[2:41 PM] <naderman> lsmith: so in a template having an "else" tag for sub requests would be cool
[2:42 PM] <naderman> but I guess that's what you meant by on-error :)
[2:42 PM] <henrikbjorn> naderman: cant be done
[2:42 PM] <johanness> what happens atm if there is a 404 exception in a sub-request?
[2:42 PM] <henrikbjorn> well not like the for twig tag
[2:42 PM] <naderman> henrikbjorn: sure, why not?
[2:43 PM] <naderman> it requires more complex ESI code hmm
[2:43 PM] <henrikbjorn> because with ESI the content would be injected into it, it will not evalutate after the tag
[2:43 PM] <naderman> it works as long as it's not standalone
[2:43 PM] <henrikbjorn> yes :)
[2:43 PM] <henrikbjorn> but essentially thats what on-error does, render a template if this request is not successful
[2:43 PM] <lsmith> johanness: afaik you have the options i mentioned .. fail the current request or ignore the subrequest
[2:44 PM] <lsmith> uhm .. "Varnish only supports the src attribute for ESI tags (onerror and alt attributes are ignored)."
[2:44 PM] <lsmith> http://symfony.com/doc/2.0/cookbook/cache/varnish.html
[2:45 PM] <henrikbjorn> thats a bommer
[2:45 PM] <naderman> henrikbjorn: there is <esi:try><esi:attempt></esi:attempt><esi:except></esi:except></esi:try> for this ;-)
[2:45 PM] <henrikbjorn> wow
[2:45 PM] <lsmith> "ignore_errors: if set to true, an onerror attribute will be added to the ESI with a value of continue indicating that, in the event of a failure, the gateway cache will simply remove the ESI tag silently."
[2:45 PM] <naderman> but I doubt varnish implements any of those
[2:45 PM] <henrikbjorn> lets not support that
[2:45 PM] <lsmith> http://symfony.com/doc/2.0/book/http_cache.html
[2:46 PM] <Stof> lsmith: varnish does not support onerror
[2:46 PM] <lsmith> Stof: yeah .. thats a pretty big issue
[2:46 PM] <lsmith> so that means we need to make it possible for render tags to easily specify what they want to happen
[2:47 PM] <naderman> henrikbjorn: varnish argues that supporting stuff like ESI attempt/except makes no sense, because you can do the fallback inside VCL configuration
[2:47 PM] <naderman> same for onerror really
[2:48 PM] <lsmith> aka there needs to be a flag to catch the exception and instead return a Reponse with an empty content
[2:48 PM] <lsmith> standard > custom config
[2:49 PM] <lsmith> i knew it .. should have perstered that varnish guy some more
[2:49 PM] <lsmith> he brushed off my question with "such stuff isnt one size fits all"
[2:51 PM] <johanness> i think removing the ability to secure controller via configuration is probably the easiest solution
[2:52 PM] <lsmith> right .. it would at least prevent false expectations
[2:52 PM] <lsmith> but it would still lead to inconsistencies between ESI and non ESI
[2:52 PM] <johanness> hm, how do esi requests look like? what path do they have?
[2:53 PM] <johanness> maybe some special attribute?
[2:53 PM] <lsmith> johanness: they use an internal route .. so the path will look differently
[2:53 PM] <lsmith> hmm ..
[2:53 PM] <lsmith> so i guess there is no inconsistency in that case
[2:53 PM] <lsmith> since you would have to explicitly also secure the ESI paths
[2:54 PM] <lsmith> well it would still be inconsistent ... but less surprising
[3:00 PM] <johanness> fabpot, what is the status of https://github.com/symfony/symfony/pull/198 ? this is kind of blocking any changes to security atm
[3:08 PM] <beberlei> lsmith, which slideset had this architecture image how the MVC request stack worked? symfony from the trenches?
[3:08 PM] <lsmith> beberlei: yes
[3:08 PM] <lsmith> i can send you the source for that .. its a google docs
[3:09 PM] <beberlei> thad would be nice, kontakt@beberlei.de that is
[3:10 PM] <beberlei> hm no
[3:10 PM] <beberlei> thats not the one i meant
[3:10 PM] <beberlei> there was a nicer one somewhere else ;)
[3:10 PM] <fabpot> johanness: the problem with this PR is that there are many things in one PR
[3:11 PM] <fabpot> I can merge the protected/private changes
[3:11 PM] <lsmith> jonwage: i will upload a new copy of the trenches slides to slideshare ok?
[3:11 PM] <fabpot> but I want more time to think about the User/Account rename
[3:11 PM] <jonwage> ok cool
[3:20 PM] <fabpot> johanness: merged