Skip to content

Conversation

roccocorsi
Copy link
Contributor

New pull request with changes suggested by Günter.

I was thinking about this over the holidays and it is obvious that this is an IPv4 only solution. I'm not an IPv6 expert at all so not really sure what would be the proper way to make it IPv4 and IPv6 compliant, and how smart to make it.

@aleks-f
Copy link
Member

aleks-f commented Jan 7, 2018

I'm ok with IPv4-only for now. If anyone has IPv6 itch, they can scratch it :)

@@ -0,0 +1,274 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since secure sockets are supported here, it properly belongs in NetSSL_OpenSSL/samples

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I can move it there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aleks, did you retrigger running the checks? I remember them being successful last time I ran them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see that you made some small changes (adding copyright notice and fixing tabs), must have retriggered the checked with that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, can you also add CMakeLists.txt and Makefile for the sample - it should be simple, just take the ones from an existing example and modify the names accordingly.

@roccocorsi
Copy link
Contributor Author

OK, I moved the SetSourceIP example to sample directory in NetSSL_OpenSSL. First time that I update a branch like this and pleasantly surprised that the checks are run automatically. I thought I had to make a new force commit at same level.

Lets hope checks pass. :-)

@roccocorsi
Copy link
Contributor Author

One of the check jobs failed due to some scripting issue.

Should I squash all changes into one commit and try again?

@aleks-f aleks-f added this to the Release 2.0.0 milestone Jan 8, 2018
@roccocorsi
Copy link
Contributor Author

Hi Aleks

I received an email with comments from you about adding files CMakeLists.txt and Makefile for SetSourceIp example program. I can't seem to find your comments on the github site, so I can't answer them directly.

Anyway, I tried to copy CMakeLists.txt from HTTPSTimeServer and make changes to it to refer to SetSourceIp where expected. Ran cmake on the file, seems to work, and it created a 'Makefile' file.

Tried to 'make' the Makefile, but I get a lot of errors when compiling related to enabling C++ 11.

Add the the following line in CMakeLists.txt
add_definitions(-std=c++11)

Everything works with that addition. Not sure if forcing c++11 is OK? When I compile other samples I get same C++ 11 issues. Do I have too old or too new cmake or g++?!?!

Any suggestions to move forward?

@roccocorsi
Copy link
Contributor Author

I've made a new commit with CMakeLists.txt and Makefile changes. Not 100% sure how to properly test them. Would appreciate feedback.

@roccocorsi
Copy link
Contributor Author

I think travis-ci might be stuck!! Is there a way to abort and restart the check jobs?

@zosrothko
Copy link
Member

@roccocorsi You can either restart the whole build or restart a single job. Click on the small icon on the right of each job or on the top left icon.

@aleks-f
Copy link
Member

aleks-f commented Jan 9, 2018

I restarted the build. As for CMake, I will look at the issue, hopefully by the end of week. Generally speaking, yes - C++11 is ok in develop branch

@roccocorsi
Copy link
Contributor Author

Sorry, been away for some time. Is there anything else that is needed from me on this commit to get it merge on develop branch?
Thank you.

@aleks-f
Copy link
Member

aleks-f commented Feb 28, 2018

d0312c6

had to add VS projects, then had upstream conflict. reset, then commit again and @roccocorsi name in commits got lost. sorry, we like people to get proper credit for their contributions, but it's too much to go through the whole cycle again

@aleks-f
Copy link
Member

aleks-f commented Feb 28, 2018

tried to amend to at least point to your contributions, ended up wth two commits and a merge, lol. anyway, thank you for the contribution :)

@roccocorsi
Copy link
Contributor Author

Glad to hear that it is all merged. No problem if my name is not part of the commit.

I might be back in a few weeks with IPv6 part, maybe...

@roccocorsi
Copy link
Contributor Author

Commit will be part of Poco 2.0.0? No back porting, right?

@aleks-f
Copy link
Member

aleks-f commented Feb 28, 2018

yes, if you need it in 1.x cherry-pick 26e952f and send a pull

@roccocorsi
Copy link
Contributor Author

Any ideas when 2.0.0 might go stable?

@aleks-f
Copy link
Member

aleks-f commented Mar 1, 2018

hard to tell, months for sure. here's the project, and there is one fairly large outstanding issue that needs to be fixed before release (first release will be beta).

If you can help, it will be faster :)

@p-neels
Copy link

p-neels commented Jul 31, 2018

Have tried the changes from this pull request but still it doesn't work.
setsockfd was the solution.
But setsockfd doesn't work straight forward.Hence have created below issue for same.
#2400

Is there any plans to expose setsock option for secure connection directly to user for a secure connection?

@gelldur
Copy link

gelldur commented Jan 25, 2024

There is issue when we close connection and try to reconnect. Such session wont use correct source address.
I found that bug is caused by:

void SecureSocketImpl::connect(const SocketAddress& address, const Poco::Timespan& timeout, bool performHandshake)

because in first line we do:

if (_pSSL) reset();

Which will recreate new socket without binding. I'm working on fix.

HTTPClientSession::reconnect()
    -> HTTPSession::connect(const SocketAddress& targetAddress, const SocketAddress& sourceAddress)
        -> _socket.bind <----- as we closed socket now we recreate it
        -> HTTPSession::connect(const SocketAddress& address)
            -> _socket.connect(address, _connectionTimeout); (this is calling: SecureSocketImpl instance)
                -> SecureSocketImpl::connect(const SocketAddress& address, bool performHandshake)
                    -> reset() <----------------- issue here as we again destroy and create docket
                    -> SocketImpl::connect(const SocketAddress& address, const Poco::Timespan& timeout)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants