Re: [Twisted-Python] twistedmatrix.com TLS certificate

[Cory Benfield]

> On 6 Mar 2017, at 07:22, Tristan Seligmann  wrote:
> 
> twistedmatrix.com 's current certificate is issued 
> by StartCom Certification Authority; for certificates issued by this CA prior 
> to 2016-09-21, the domain must be on a Chrome whitelist for it to be 
> accepted. As of Chrome 58.0.3026.3 (canary/dev channel only, currently, but 
> eventually this will presumably be in a release version) twistedmatrix.com 
>  is no longer[1] on the whitelist, which means 
> that twistedmatrix.com  will issue a certificate 
> error. Can we switch to another CA? (Let's Encrypt, for example; I hear 
> somebody wrote a Twisted library for using that)
> 
> I'm sending this to the general list in case anyone else has been scratching 
> their head about why they're getting cert warnings.

This is an extremely good idea.

Cory

___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] HTTP Agent persistent connections not closed for some HTTPS sites

[Cory Benfield]

> On 5 Mar 2017, at 14:25, Tristan Seligmann  wrote:
> 
> On Sun, 5 Mar 2017 at 15:36 Adi Roiban  > wrote:
> I have observed this while running some end to end tests in which the
> pool.closeCachedConnections() deferred was not called, even after a
> generous amount of seconds :)
> 
> The code to abort an HTTP client connection is here:
> 
> https://github.com/twisted/twisted/blob/twisted-17.1.0/src/twisted/web/_newclient.py#L1657
>  
> 
> 
> This calls loseConnection which for a TLS connection will try to do a TLS 
> shutdown under most circumstances (in some cases it can't, and will 
> abortConnection on the underlying transport instead). If the remote end has 
> stopped responding to the connection, I think this may end up hanging forever.
> 
> I think this code should either call abortConnection directly, or set a timer 
> which will abort the connection after a little while if a clean shutdown from 
> loseConnection has not completed yet. I’

Yeah, this looks right. We hit it in the server side code too: it’s a very 
unintuitive API in that sense.

Cory

___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] HTTP Agent persistent connections not closed for some HTTPS sites

[Adi Roiban]

On 6 March 2017 at 08:17, Cory Benfield  wrote:
>
> On 5 Mar 2017, at 14:25, Tristan Seligmann  wrote:
>
> On Sun, 5 Mar 2017 at 15:36 Adi Roiban  wrote:
>>
>> I have observed this while running some end to end tests in which the
>> pool.closeCachedConnections() deferred was not called, even after a
>> generous amount of seconds :)
>
>
> The code to abort an HTTP client connection is here:
>
> https://github.com/twisted/twisted/blob/twisted-17.1.0/src/twisted/web/_newclient.py#L1657
>
> This calls loseConnection which for a TLS connection will try to do a TLS
> shutdown under most circumstances (in some cases it can't, and will
> abortConnection on the underlying transport instead). If the remote end has
> stopped responding to the connection, I think this may end up hanging
> forever.
>
> I think this code should either call abortConnection directly, or set a
> timer which will abort the connection after a little while if a clean
> shutdown from loseConnection has not completed yet. I’
>
>
> Yeah, this looks right. We hit it in the server side code too: it’s a very
> unintuitive API in that sense.

I am not sure about which code are we talking here.
The specific HTTP11ClientProtocol which will fix only the HTTP client
part or the generic TLSMemoryBIOProtocol code which might fix any TLS
connection?

I have observed this issue on the server side for the case in which a
TLS connection is opened and then closed  without transferring any
data ... and then the handshake will not have the chance to complete.

I think that this is the same issue which was reported for Scrapy

-- 
Adi Roiban

___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

[Twisted-Python] non-merge commits to trunk & "review" keyword

[Jean-Paul Calderone]

Hello,

GitHub apparently allows fast-forward merges to trunk.  Here's an example
of one:

  https://github.com/twisted/twisted/pull/730

This doesn't seem like a good thing.

   - The ticket is still open
   - There is no merge commit
   - There is no merge commit message
   - There are non-merge commits directly on trunk history (first parent)

  Anyone else have an opinion?

Also, on the same PR, it seems like folks have trouble managing the two
different ways to represent the review states: the "review" keyword in trac
and the accepted/etc status on the GitHub PR.  Maybe there shouldn't be two
different ways to represent this?

Jean-Paul
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] twistedmatrix.com TLS certificate

[Glyph Lefkowitz]

> On Mar 6, 2017, at 12:16 AM, Cory Benfield  wrote:
> 
> 
>> On 6 Mar 2017, at 07:22, Tristan Seligmann > > wrote:
>> 
>> twistedmatrix.com 's current certificate is 
>> issued by StartCom Certification Authority; for certificates issued by this 
>> CA prior to 2016-09-21, the domain must be on a Chrome whitelist for it to 
>> be accepted. As of Chrome 58.0.3026.3 (canary/dev channel only, currently, 
>> but eventually this will presumably be in a release version) 
>> twistedmatrix.com  is no longer[1] on the 
>> whitelist, which means that twistedmatrix.com  
>> will issue a certificate error. Can we switch to another CA? (Let's Encrypt, 
>> for example; I hear somebody wrote a Twisted library for using that)
>> 
>> I'm sending this to the general list in case anyone else has been scratching 
>> their head about why they're getting cert warnings.
> 
> This is an extremely good idea.

Yes please.

This is the rare ops task that will actually be quite easy for someone to add 
in to Braid as a PR: https://github.com/twisted-infra/braid 


If you have a look at 
https://github.com/twisted-infra/braid/blob/master/services/t-web/twisted-web/ports
 

 you might be able to guess how such a thing would go...

-glyph

___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] non-merge commits to trunk & "review" keyword

[Glyph Lefkowitz]

> On Mar 6, 2017, at 6:03 PM, Jean-Paul Calderone  
> wrote:
> 
> Hello,
> 
> GitHub apparently allows fast-forward merges to trunk.  Here's an example of 
> one:
> 
>   https://github.com/twisted/twisted/pull/730 
> 
> 
> This doesn't seem like a good thing.
> The ticket is still open
> There is no merge commit
> There is no merge commit message
> There are non-merge commits directly on trunk history (first parent)
>   Anyone else have an opinion?

This is definitely bad, forbidden by existing policy, etc.  In fact I remember 
adjusting the settings so that the 'merge' button would always create a merge 
commit; in fact, the configuration is still set that way.

Craig, since you're the one who made this merge, can you explain what happened? 
 Has github's 'merge' button stopped prompting for a commit message?  Failing 
to wait for the removal of the 'review' keyword was obviously a mistake, but 
based on my previous experience with the 'merge' button this wouldn't even be a 
type of mistake that's _possible_ to make; understanding what you did to 
trigger it would therefore be very useful.

I don't think this is bad enough to do any major VCS surgery to fix (similar 
mistakes exist in the history, and both commits to Twisted.Quotes and literally 
all of the imported SVN history are single-parent), but it is worth doing some 
work to avoid recurring.

> Also, on the same PR, it seems like folks have trouble managing the two 
> different ways to represent the review states: the "review" keyword in trac 
> and the accepted/etc status on the GitHub PR.  Maybe there shouldn't be two 
> different ways to represent this?

Indeed there should not.  I hope someone has the time to address this soon

The only blocker is that I don't want to lose the notion of a "review queue".  
The random mess of works-in-progress, already-reviewed code awaiting feedback, 
and abandoned experiments that shows up on the default 'pull request' view is 
not a workable substitute for the relatively short and explicit "I have 
submitted code that wants a review but hasn't gotten one" list at 
https://twisted.reviews/ .  However, some combination 
of Github's new-ish explicit review workflow, (possibly, if necessary) some 
kind of bot to perform the operations that are mysteriously forbidden to 
outside contributors, and a custom query could easily do the trick if someone 
can work it out and document it.

-glyph

___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] non-merge commits to trunk & "review" keyword

[Tristan Seligmann]

On Tue, 7 Mar 2017 at 06:38 Glyph Lefkowitz  wrote:

>
> This is definitely bad, forbidden by existing policy, etc.  In fact I
> remember adjusting the settings so that the 'merge' button would always
> create a merge commit; in fact, the configuration is still set that way.
>

Rebase merging was allowed when I checked earlier, so I disabled it (I
guess I managed to do this before you looked at the configuration). I
suspect that is what happened here, but note also that these settings only
control what is possible by pressing the merge button on GitHub; you can
construct commits locally however you like and push them, as long as you
get green commit statuses for all the mandatory commit checks.
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] non-merge commits to trunk & "review" keyword

[Glyph Lefkowitz]

> On Mar 6, 2017, at 9:02 PM, Tristan Seligmann  wrote:
> 
> On Tue, 7 Mar 2017 at 06:38 Glyph Lefkowitz  > wrote:
> 
> This is definitely bad, forbidden by existing policy, etc.  In fact I 
> remember adjusting the settings so that the 'merge' button would always 
> create a merge commit; in fact, the configuration is still set that way.
> 
> Rebase merging was allowed when I checked earlier, so I disabled it (I guess 
> I managed to do this before you looked at the configuration).

Perhaps I only did this for some other repos, then.  Did I miss your 
announcement of this?  Hint, hint? :)

> I suspect that is what happened here, but note also that these settings only 
> control what is possible by pressing the merge button on GitHub; you can 
> construct commits locally however you like and push them, as long as you get 
> green commit statuses for all the mandatory commit checks.

Is this true even for people with just repo:write?

-glyph

___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python