Changes by Demian Brecht demianbre...@gmail.com:
Added file: http://bugs.python.org/file37309/issue22095_1.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22095
Demian Brecht added the comment:
Thanks Serhiy, new patch addresses your comments.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22095
Demian Brecht added the comment:
There could be some history behind this that I'm unaware of that I'm not
familiar with.
From what I can tell, this issue is simply due to the [ character not being
in _LegalCharsPatt (http/cookies.py). _LegalCharsPatt actually seems quite a
bit more
Demian Brecht added the comment:
Left a comment in Rietveld.
--
nosy: +demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21557
Demian Brecht added the comment:
It's generally better to keep everything in one issue in order to preserve
history and avoid confusion as you've now found :). I'd suggest closing this
one and adding the patch to the initial report. Keeping history intact is
important for reviewers
Demian Brecht added the comment:
Err, sorry, I entirely misunderstood the problem. The invalid characters are
correct ([ = 5B, which indeed is illegal, I wasn't paying close enough
attention to the hex values in the ABNF). It's the fact that the valid
key/value pairs after the invalid one
Demian Brecht added the comment:
Now I've confused myself and my first impression was correct. For some reason,
my brain was thinking %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E was the
exclusion list for some reason (which is obviously horribly wrong).
So my first observation was correct
Demian Brecht added the comment:
Attached patch to fix the issue as reported.
Something interesting that came out of this though is that due to the regex
expression, if there's an invalid character in one of the cookie-octets, the
rest of the cookie is ignored. I would assume that it should
New submission from Demian Brecht:
As found in #22931, if an invalid cookie value is found while parsing, the rest
of the cookie is silently ignored. The expected behavior is undefined in RFC
6265, but does state that if unexpected values are encountered that user agents
MAY ignore an entire
Demian Brecht added the comment:
I do think it should be a little more permissive when parsing cookies. I've
created #22983 to address that as to not conflate this issue, which the
attached patch does address.
--
___
Python tracker rep
Demian Brecht added the comment:
Sure, but this is in regards to the initial matching, not the parsing. Because
the pattern expects RFC conformity, in this cookie string:
Cookie: a=b; c=[; d=r; f=h
The only matching parts will be:
Cookie: a=b;
The rest will be discarded. What I'm proposing
Demian Brecht added the comment:
Sorry, bad example. Replace [ in the previous example with any actually
invalid character.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22983
Demian Brecht added the comment:
After discussion in Rietveld, the patch looks good to me.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21557
New submission from Demian Brecht:
Coming out of a recent thread in python-dev, it was mentioned that adding a git
developer's guide to mercurial may be beneficial to help lower the initial
barrier of entry for prospective contributors
(https://mail.python.org/pipermail/python-dev/2014
Demian Brecht added the comment:
Thanks for the comments, will try to have an updated patch today.
Trying to learn usual hg workflows might also be easier than trying to use a
git-like workflow on mercurial, so you could suggest considering them too (I
don't remember if we have anything
Demian Brecht added the comment:
Bump (again) for hopeful merge.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
___
Python-bugs-list
Demian Brecht added the comment:
I believe most points should be addressed in the updated patch. Below are
responses to other comments (those not included have resulted in changes in the
patch).
@Ezio:
4) it should be possible to use evolve instead of bookmarks, but evolve is
currently
Demian Brecht added the comment:
Updated patch addressing review comments. Thanks for the review.
--
Added file: http://bugs.python.org/file37368/issue21793_3.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
Demian Brecht added the comment:
+1 to the addition of a test. Also, I know it's only related, but it might be
nice to also have this fixed in http_error_default:
https://hg.python.org/cpython/file/021c1df36910/Lib/urllib/request.py#l2003.
--
nosy: +demian.brecht
Changes by Demian Brecht demianbre...@gmail.com:
Added file: http://bugs.python.org/file37370/issue21793_4.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
Demian Brecht added the comment:
Contributors may need to update their repos before this step. It would be
good to add a note about git pull --rebase.
Working through this flow actually exposed a blocking issue as far as this
guide goes. As I understand it, because Mercurial sends hashes
Demian Brecht added the comment:
One option would be to increase LimitRequestFieldSize in Apache, but that
really is only a stopgap solution as that limit will be hit again as one's
local repo grows over time. It really would be nice to allow read only access
via SSH, which isn't affected
Demian Brecht added the comment:
Did you actually hit this issue with hg.python.org?
Yes I did, with only one additional head:
https://gist.github.com/demianbrecht/f525cceda8a0c99794ae. This would also lead
me to believe that the public at large will also start hitting
Demian Brecht added the comment:
After some further investigation, this seems to be what's happening. Note: this
is based on a small amount of research and hasn't been validated by any
mercurial devs, so it might be missing key pieces: In cases where a local
topological head is unknown
Demian Brecht added the comment:
The issue and solution was confirmed on IRC in #mercurial by mpm (Matt
Mackall). According to him, there is also no easy way to change this behaviour
from the client. The problem isn't /really/ a Mercurial issue, but an issue
with web server specific defaults
Demian Brecht added the comment:
I've added a section about rebasing and removed the comment about merging. Of
course, I'm assuming that the config changes can be made to Apache to allow for
this workflow.
--
Added file: http://bugs.python.org/file37412/issue22992_1.patch
Demian Brecht added the comment:
Coming back to this, it's likely a bad idea as an addition to the standard
library as an update may be required out of band with scheduled deployments
should schemas and such change. This is best developed as an external module.
Closing.
--
status
Demian Brecht added the comment:
Thanks for the ping Serhiy, indeed the review notification email was sitting in
spam. New patch addressing review comments as well as rectifying my own
silliness.
--
Added file: http://bugs.python.org/file37419/issue22095_2.patch
Demian Brecht added the comment:
Self review/update: removed two errant breakpoints and updated the what's new
section (missed in my previous patch).
--
Added file: http://bugs.python.org/file37420/issue21793_5.patch
___
Python tracker rep
Demian Brecht added the comment:
The issue was actually HAProxy rather than Apache (I had misread a line in the
documentation so my understanding of Apache header limitations was incorrect).
This was fixed by Donald today. If there are no objections, this should be good
to merge now
Demian Brecht added the comment:
Updated patch addressing further reviews
--
Added file: http://bugs.python.org/file37434/issue21793_6.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
Demian Brecht added the comment:
Thanks for the patch Aaron. Unfortunately this doesn't quite fix the issue.
There are two problems with the patch:
If a bytes object is passed into mock_open, I'd expect a bytes object in the
output. In your patch, not only is this not the case (the output
Demian Brecht added the comment:
There are two problems with the patch
That was intended to be removed after I changed the wording :P
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23004
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: +demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23043
___
___
Python-bugs
Demian Brecht added the comment:
Thanks for the update, but this doesn't quite work either as you're assuming
utf-8 (which is what .encode() and .decode() default to). For example, when
using latin-1:
m = mock_open(read_data= b'\xc6')
with patch('__main__.open', m, create=True
Demian Brecht added the comment:
Thanks again for the update Aaron, I've left a couple small comments in
Rietveld. Other than those, the patch looks good to me. Thanks for the
contribution!
--
___
Python tracker rep...@bugs.python.org
http
Demian Brecht added the comment:
I'm a -1 to adding the timeout parameter to the ServerProxy implementation for
pretty much the same reasons Jeff mentioned, but mainly because of the
ambiguity that is introduced between the timeout and transport parameters (who
should win in the case
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: +demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22928
___
___
Python-bugs
Demian Brecht added the comment:
+ loewis as he's listed as the xmlrpc expert
If you're worried about the number of lines, turn the function into a lambda:
proxy = ServerProxy('http://example.com/gateway/', transport=Transport(
connection_factory=lambda h: HTTPConnection(h, timeout
Demian Brecht added the comment:
Thanks for the updated patch, looks good to me. If you haven't already read it,
the patch workflow is here: https://docs.python.org/devguide/patch.html and is
the only workflow currently available.
--
___
Python
Demian Brecht added the comment:
socket.setdefaulttimeout([timeout]) -- it is bad practice
I'm not really arguing this. It solves the problem, but definitely not in the
best of ways. My point in referencing setdefaulttimeout is that if /all/ you
care about is the timeout and you're horribly
Demian Brecht added the comment:
in GNU/Linux system timeout has been reached -- means that system timeout
will *never* reached.
That's quite likely because the system limits may be very large. For example,
on my OSX box:
--- ~ » sysctl net.inet.tcp.keepinit
net.inet.tcp.keepinit
Demian Brecht added the comment:
I think we've started to venture into system-level territory that the standard
library itself shouldn't have to account for. If TCP on systems are configured
by default to allow for infinite timeouts, then it should likely be an issue
for those distros. I
Demian Brecht added the comment:
On another note, running a simple test with against a non-routable IP yields
that OSX's default timeout is 75 seconds and not 7500 seconds as the developer
docs lead me to believe.
--
___
Python tracker rep
Demian Brecht added the comment:
I've attached a test-less patch with my suggested approach. If there's no
opposition to this change, I'll put some work into getting tests done for it as
well.
--
Added file: http://bugs.python.org/file37481/issue14134.patch
Demian Brecht added the comment:
@Julien.Palard: There's a subtle difference between your test and the issue as
written. Your test lives within a module and therefore executes testmodule (see
https://hg.python.org/cpython/file/9f60d024e586/Lib/doctest.py#l1819) whereas
the issue reported uses
Demian Brecht added the comment:
it's been resolved in 3.5
Sorry, that statement can be a little misleading, possibly indicating that
something may have changed in the doctest globals handling. It was resolved in
3.5 because print is no longer a statement so this ambiguous behaviour resolved
Demian Brecht added the comment:
The patch in #21793 has been merged, resolving this issue as well. This should
now be closed.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20898
Demian Brecht added the comment:
A few more comments were left in Rietveld for you, likely hidden by spam
filters.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23004
Demian Brecht added the comment:
Ping
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22992
___
___
Python-bugs-list mailing list
Unsubscribe
Demian Brecht added the comment:
@Berker: Good point, although I think that the status code table in
http.client.rst should be merged with the one in http.rst as to avoid
redundancy (newly added status codes should also have links added). The table
in http.client.rst should likely be replaced
Demian Brecht added the comment:
A few notes:
1. Unicode hosts are not automatically IDNA-encoded (which they /could/ be
rather than relying on the programmer to be aware of this), but this really has
no bearing on this specific issue
2. Unicode paths are not automatically IRI-encoded (see
Demian Brecht added the comment:
Thanks for the work! I'm not sure why the last patch doesn't appear on
Rietveld, so (unfortunately) here's the result of my review. I've only covered
functional aspects in this run at it:
+base_files = ['index.html', 'index.htm']
Can you use index_files
Demian Brecht added the comment:
@demian: If you don't mind, could you please elaborate a bit more on
`_resolve_path()` you mentioned in the review/comment?
Sure. In your patch, you have redirect_browser (or redirect if you
renamed it), which sounds like it's allowing for a very generic
Demian Brecht added the comment:
Thanks for the extra effort on this to satisfy multiple people's opinions. It's
never an easy thing, especially when dealing with a decentralized group. My
following comments are largely based on the public API. I haven't done a full
review of the changes made
Demian Brecht added the comment:
now I’m thinking that should be up to the higher level user
+1. A closed connection is a closed connection, whether it's persistent or not.
The higher level code should be responsible for the context, not the connection
level
Demian Brecht added the comment:
Sorry Martin, I should really not dig into issues like this first thing in the
morning ;)
My concern about the proposed change isn't whether or not it isn't valid HTTP
behaviour, it is. My concern (albeit a small one) is that the change implies an
assumption
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: +demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23255
___
___
Python-bugs
Demian Brecht added the comment:
No worries, thanks for taking care of merging it Berker.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20898
Demian Brecht added the comment:
Calling self.wfile.write(b) should be equivalent to not calling write() at
all, as far as I understand.
Right (or at least, as I understand it as well).
Really, this boils down to a philosophical debate: Should the standard library
account for unexpected
Demian Brecht added the comment:
TL;DR: Because HTTP is an application-level protocol, it's nearly impossible to
gauge how a server will behave given a set of conditions. Because of that, any
time that assumptions can be avoided, they should be.
@R. David Murray:
That is, if the connection
Demian Brecht added the comment:
Now I think I'd like to take my foot out of my mouth.
Previous quick experiments that I had done were at the socket level,
circumventing some of the logic in the HTTPResponse, mainly the calls to
readline() rather than simple socket.recv(N).
I've confirmed
Demian Brecht added the comment:
I'm not sure whether or not this was your intention, but your example
demonstrates a misbehaving client, one that seems to expect a persistent
connection from a non-persistent server. TCPServer will only serve a single
request and will shutdown and close
Demian Brecht added the comment:
Hi Martin,
Thanks for the example code. I'm not sure whether or not this was your
intention, but your example demonstrates a misbehaving client, one that seems
to expect a persistent connection from a non-persistent server. TCPServer will
only serve a single
Demian Brecht added the comment:
Ping. Would be nice to get this change in before 3.5.0a1.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20898
Demian Brecht added the comment:
@Senthil: If you're fixing this today, can you also correct the typo here:
https://hg.python.org/cpython/rev/568041fd8090/#l1.27 (s/HTML/HTTP)? Just
caught my eye going through the review that Mark linked.
--
nosy: +demian.brecht
Demian Brecht added the comment:
I'm not opposed to that either. The only downside really (at least as far as
I'm aware) is the potential substantial influx of packets should an iterable
comprised of small chunks of data be passed in as the body, although I would
consider that quite a strange
Demian Brecht added the comment:
(Admittedly, I may also have been doing something entirely invalid in previous
experiments as well)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue3566
Demian Brecht added the comment:
This should likely be closed as a duplicate of #3566, which has additional
detail.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8450
Demian Brecht added the comment:
Trying to reproduce this on my own in 3.5, 2.7 and 2.5 yields a
ConnectionResetError (ECONNRESET), which seems to be correct. That said, this
could be due to varying TCP implementations on the server so might still be
valid. It could also be due to an older
New submission from Demian Brecht:
There are a couple of small issues with the determination of whether or not a
request can fit in a single TCP/IP packet in http.client.
1. The MSS is hardcoded
2. The TCP data size is calculated as only the message body. This is incorrect
as the size
Changes by Demian Brecht demianbre...@gmail.com:
--
components: +Library (Lib)
keywords: +patch
versions: +Python 3.5
Added file: http://bugs.python.org/file37822/issue23302.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org
Demian Brecht added the comment:
Thanks for helping with this Demian.
No worries. This textual white boarding exercise has also been greatly
valuable in getting my own head wrapped around various low frequency
socket level errors that can be encountered when using the client. The
downside
Demian Brecht added the comment:
My apologies for the delay, but I've now reviewed the proposed patch. With a
fresh outlook after taking a bit of time off, I'm not sure anymore that this is
the best way of going about solving this problem. The main reason being that we
now have two errors
Demian Brecht added the comment:
I’ve reverted the patch to use the old format. The main reason being that plain
ints can still be used in most cases as values for code, in which case logging
will be inconsistent with cases using the enum.
--
Added file: http://bugs.python.org
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: +demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23448
___
___
Python-bugs
Demian Brecht added the comment:
Thanks for the catch Martin, the field should indeed be updated to be
plural. I'll try to get a patch for this later today unless someone else
beats me to it
--
___
Python tracker rep...@bugs.python.org
http
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: +demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23439
___
___
Python-bugs
Demian Brecht added the comment:
The only real question I have is: why? As far as I'm aware, these are
implementation details of the http.client module (there's even a comment in
HTTPMessage that it might make sense to move the class altogether).
As far as the constants go, they're only
Demian Brecht added the comment:
@Mark: Sure, but not super high priority. Thanks for pointing it out.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue2211
Demian Brecht added the comment:
Maybe join them with tabs rather than spaces then, since it was previously
\r\n\t. This way it is even closer to before.
After thinking about this a little more, I think I'd prefer to keep
spaces rather than tabs. The reason being is that, in my mind, now
Demian Brecht added the comment:
Some refactoring that I'm working on for http.client could use this (currently
I have it as part of my patch set). I haven't run into any issues using it and
it's definitely useful. Would be nice to get this merged.
--
nosy: +demian.brecht
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: +demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21257
___
___
Python-bugs
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: +demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue5053
___
___
Python-bugs
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: +demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue5054
___
___
Python-bugs
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: +demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9698
___
___
Python-bugs
Demian Brecht added the comment:
Thanks for the test Berker, I'll put a patch together with the changes
later this afternoon.
On 2015-02-12 2:27 PM, Berker Peksag wrote:
Berker Peksag added the comment:
Here is a test case
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: -demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22147
___
___
Python-bugs
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: -demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12455
___
___
Python-bugs
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: -demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17908
___
___
Python-bugs
Demian Brecht added the comment:
If we leave it as it is, it would be good to add comment in the source code
explaining this decision.
I think that __all__ should be left as-is for the time being. Adding
some comments around that decision makes sense to me to avoid any future
confusion around
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: -demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22197
___
___
Python-bugs
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: -demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14044
___
___
Python-bugs
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: -demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14414
___
___
Python-bugs
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: -demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23043
___
___
Python-bugs
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: -demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23004
___
___
Python-bugs
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: -demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23448
___
___
Python-bugs
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: -demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23166
___
___
Python-bugs
Demian Brecht added the comment:
I've attached a patch with fixes for both cases and the tests added by
Berker. Thanks guys.
--
Added file: http://bugs.python.org/file38122/issue23442_1.patch
___
Python tracker rep...@bugs.python.org
http
Changes by Demian Brecht demianbre...@gmail.com:
--
nosy: -demian.brecht
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22946
___
___
Python-bugs
201 - 300 of 537 matches
Mail list logo
