-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Close the connection used by https #256
Conversation
Can you write a test demonstrating that? So that it fails before your commit and succeed afterwards? |
@elazarl, I can confirm this issue. I created a test for this (on linux) that will fail before the commit and succeed afterwards: https://gist.github.com/oec/34d6acf1457b779ed7e446ddf3275762. |
Can you please add it to the PR?
…On Fri, Aug 31, 2018, 16:02 Özgür Kesim ***@***.***> wrote:
@elazarl <https://github.com/elazarl>, I can confirm this issue. I
created a test for this (on linux) that will fail before the commit and
succeed afterwards:
https://gist.github.com/oec/34d6acf1457b779ed7e446ddf3275762.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#256 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAP4ou4SGr5qZAomVEZ-rGttO5poiYJmks5uWTQAgaJpZM4QV1Pr>
.
|
That would be @YushiIso's job, I suppose, as he owns the PR. |
If you can send it as a difference PR, I'll merge both
…On Sun, Sep 2, 2018, 21:27 Özgür Kesim ***@***.***> wrote:
That would be @YushiIso <https://github.com/YushiIso>'s job, I suppose,
as he owns the PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#256 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAP4oo3FK19ka1sfSfpu1SWjcu35vpDIks5uXCMIgaJpZM4QV1Pr>
.
|
@oec Thanks!! |
Thanks.
Would merge later today.
…On Mon, Sep 3, 2018, 04:55 YushiIso ***@***.***> wrote:
@oec <https://github.com/oec> Thanks!!
Added test to the PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#256 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAP4oq3aXRj0yMPQ6uNO9c9_URR3p-zaks5uXIwLgaJpZM4QV1Pr>
.
|
@elazarl, when is the best moment for the merge, if not now? :) |
Important changes: Fixes elazarl#256: connections are closed after usage, so FD-count remains congruent with the actual amount of open connections. Added ProxyHttpServer.Signer of type type Signer func(ca *tls.Certificate, hostname []string) (*tls.Certificate, error) If ProxyHttpServer.Signer is set, the goproxy uses that function to retrieve Certificates for TLS-interception. This allows consumers of goproxy to implement f.e. caching of certificates. Minor changes: - adjusted import-path to github.com/oec/goproxy - go fmt on all files
merging with elazarl#256
a socket of the connection created by https request remains.
Read and Write Connection will be shutdown if the connection is rebuilt or the connection is terminated.
however, the socket used by connection is not closed.
you can find it by using the "lsof" command.
the remaining connection is closed by time out, but I think that it is a problem if "high traffic" or "many clients use it".
it is better to close the used connection.