let me rephrase: I sent a patch for lazyness including also the 
content-type fix ^_^ 
lets see what @massimo thinks of it, I think this is a good occasion to 
refactor lots of bits and pieces added from time to time in a more general 
and consistent way

On Friday, August 2, 2013 6:37:52 PM UTC+2, Jonathan Lundell wrote:
>
> On 2 Aug 2013, at 9:19 AM, Niphlod <nip...@gmail.com <javascript:>> 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 <massimo....@gmail.com> 
>> 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 <nip...@gmail.com> 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 <nip...@gmail.com> 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 <nip...@gmail.com> 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 web2py+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to