<div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 9, 2019 at 4:45 PM Karthikeyan <<a href="mailto:tir.karthi@gmail.com">tir.karthi@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">I
 would recommend fixing it since it's potentially remote code execution 
on systems like Redis (latest versions of Redis have this mitigated) 
though I must admit I don't fully understand the complexity since there 
are multiple issues linked. Go was also assigned a CVE for linked issue 
and it seemed to be the same reporter by username : CVE-2019-9741 . I 
tried using go's approach in the commit but urlopen accepts more URLs 
like data URLs [0] that seemed to accept \n as a valid case and the 
patch broke some tests. Looking at the issue discussion complexity also 
involves backwards compatibility. golang also pushed an initial fix that
 seemed to broke their internal tests [0] to arrive at a more simpler 
fix.<br></div><div dir="ltr"><br></div><div dir="ltr">[0] <a href="https://github.com/python/cpython/blob/a40681dd5db8deaf05a635eecb91498dac882aa4/Lib/test/test_urllib.py#L482" target="_blank">https://github.com/python/cpython/blob/a40681dd5db8deaf05a635eecb91498dac882aa4/Lib/test/test_urllib.py#L482</a></div><div dir="ltr">[1] <a href="https://go-review.googlesource.com/c/go/+/159157/2#message-39c6be13a192bf760f6318ac641b432a6ab8fdc8" target="_blank">https://go-review.googlesource.com/c/go/+/159157/2#message-39c6be13a192bf760f6318ac641b432a6ab8fdc8</a><br></div></div></div></div><br>-- <br><div dir="ltr" class="gmail-m_-1667225835853067281gmail_signature"><div dir="ltr">Regards,<div>Karthikeyan S</div></div></div></div></blockquote><div><br></div><div>useful references, thanks!  limiting the checks to only http and https as those are the text based protocols with urls transmitted in text form makes sense and avoids the data: test failures.</div><div><br></div><div>proposed simple fix in <a href="https://github.com/python/cpython/pull/12755">https://github.com/python/cpython/pull/12755</a></div><div><br></div><div>but tests are needed as is an audit of the code to see where else we may potentially need to do such things.</div><div><br></div><div>-gps</div></div></div></div>