On 2 Aug 2013, at 9:19 AM, Niphlod <[email protected]> wrote: > hold still a few hours, I'm going to submit a patch for request that uses > lazy evaluation of vars (ala web3py): should be a good occasion to do a > general cleanup of all those bits !?
No reason not to hold off, but content_type can't be lazy. BTW, I think there's another minor bug in the is_json logic: the seek(0) call should be *after* the entire try/except. We want to allow rereading the content regardless of whether there was a load exception. Also, this might be a good opportunity for var laziness, depending on how it works. For json-rpc apps like mine, parsing incoming application/json payloads into vars is a complete waste of time. > > On Friday, August 2, 2013 4:12:23 PM UTC+2, Jonathan Lundell wrote: > On 2 Aug 2013, at 12:11 AM, Massimo Di Pierro <[email protected]> wrote: >> Our policy is that request.env is just the wsgi environment, without >> computed variables. > > Except for fixup_missing_path_info. > >> Perhaps this? >> >> if not request.env.content_type and request.env.http_content_type: >> request.content_type = request.env.http_content_type >> else: >> request.content_type = request.http_content_type > > Are you suggesting a new request variable to hold content_type? > > I don't think we really need to do that, and regardless that's not the right > logic. The server is not required to give us env.http_content_type (nor *any* > content_type if there's no content). If we really, really want > request.content_type: > > if request.env.content_type: > request.content_type = request.env.content_type > elif request.env.http_content_type: > request.content_type = request.env.http_content_type > > > There are two issues here. > > 1. web2py has a bug: it's using env.http_content_type to set is_json, and it > should be using env.content_type. That's because the server is required to > give us env.content_type (if there's content; note that we don't get > env.content_type for a GET), but is not required to give us > env.http_content_type. The fix is easy; just change the is_json line to use > the right variable. > > wrong: is_json = env.get('http_content_type', '')[:16] == > 'application/json' > right: is_json = env.get('content_type', '')[:16] == 'application/json' > or: is_json = env.get('http_content_type', > '').startswith('application/json') > (because I don't like magic numbers) > > > 2. Phantom issue: should we try to anticipate servers that do not behave as > they're required to do, that is, give us env.http_content_type but not > env.content_type? We don't actually know that such servers exist; hopefully > not. However, if it *did* happen (we get env.http_content_type and not > env.content_type), then it's obvious what to do. So do we do it proactively? > > We do it already for fcgi in fixup_missing_path_info, and that may be a > (policy) mistake. It's obscure, there's no good way of testing it, and we > don't know whether there's a single web2py installation using a broken server > that doesn't have path_info. But we're sorta stuck with it, because taking it > out might break something, somewhere (maybe it should have gone in > fcgihandler in the first place). > >> >> >> >> >> >> On Thursday, 1 August 2013 14:44:30 UTC-5, Jonathan Lundell wrote: >> On 1 Aug 2013, at 12:30 PM, Niphlod <[email protected]> wrote: >>> ok. so to be on the safe side if env.http_content_type and >>> env.http_content_length are provided gluon.main should update the env >>> accordingly, and then the code can happily always use env.content_length >>> and env.content_type >> >> That would be the idea. I don't actually like the extra complication, but >> the thought that somebody might be relying on bogus behavior makes me just >> *slightly* nervous. >> >> I'd either to this (pseudo-code): >> >> if not env.content_type and env.http_content_type: >> env.content_type = env.http_content_type >> >> ...and so on. That is, don't touch variables that the server has already set. >> >> I wouldn't argue to hard for not doing that, though, esp. if Massimo's OK >> with leaving it out. Which would mean just changing our is_json test to look >> at content_type. (I scanned the rest of the source, and that seems to be the >> only place this happens.) >> >> >>> >>> On Thursday, August 1, 2013 9:21:28 PM UTC+2, Jonathan Lundell wrote: >>> On 1 Aug 2013, at 12:11 PM, Niphlod <[email protected]> wrote: >>>> ok, thanks for the additional explanation. >>>> >>>> tl;dr: As we don't "want to support" any breaking-spec servers (+1 on >>>> that), the only thing to take care of is to rely for both content-type and >>>> content-length headers to be directly on env and not expecting them to be >>>> neither http_content_length nor http_content_type. >>>> >>>> did I get that clear ? >>> >>> Yes. >>> >>> I'm not sure I entirely agree about broken servers, though. Paraphrasing >>> Postel's Law, ""Be conservative in what you send, be liberal in what you >>> accept." >>> >>>> >>>> On Thursday, August 1, 2013 9:03:34 PM UTC+2, Jonathan Lundell wrote: >>>> On 1 Aug 2013, at 11:51 AM, Niphlod <[email protected]> wrote: >>>>> @derek and @dhmorgan: actually what Iceberg posted is fine, it's really a >>>>> subtle bug that needs to be addressed as per the docs posted by out own >>>>> omniscient Jonathan, that can happen with some particular (although >>>>> allowed) server architectures. >>>>> >>>>> @jonathan: before diving in rocket's own "patching of spec-breaking >>>>> servers", is there any other header we need to address ? >>>>> >>>>> >>>> >>>> content_size is the other one in this category. >>>> >>>> A clarification, though: Rocket is not patching spec-breaking servers; >>>> it's just a server complying with the spec, which mandates content_type if >>>> the client has supplied one (which would optionally appear as >>>> http_content_type). >>>> >>>> A spec-breaking server would be one that does not include content_type >>>> when one is provided by the client. >>>> >>>> The bug is that web2py relies on http_content_type, even though the spec >>>> does not require the server to include it. >>>> >>>> My comment about working around a spec break is purely hypothetical, and >>>> applies to the case where the client provides Content-Type, and the server >>>> passes that along as http_content_type (as it should, but is not required >>>> to do) and does not also pass it as content_type (which it *is* required >>>> to do). >>>> >> >> >> >> -- >> > > > > > -- > -- --- You received this message because you are subscribed to the Google Groups "web2py-users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
