search

schuster / racket-irc

Racket IRC library
MIT License
15 stars 7 forks source link

Add TLS support #25

Closed winny- closed 8 years ago

winny- commented 8 years ago

Looks like it works. I thought it'd be a lot more work. Maybe I'm missing something? This addresses #24.

Let me know what you think. By the way how do I regenerate irc/docs?

schuster commented 8 years ago

Thanks! I'm really busy right now with work and preparing for holiday travels, but I'll try to take a look in the next couple of weeks - should just be a quick check that everything works.

As far as generating docs, you should be able to just run raco setup irc at the command line, assuming the package is installed, then raco doc irc to bring up the docs in your browser.

schuster commented 8 years ago

Hmm, I get the following error when I try to run the included echobot example with SSL enabled. I don't have time to look into it tonight myself. Any thoughts on your end?

ssl-connect: connect failed (error:140770FC:SSL routines:SSL23_GET_SERVER_HELLO:unknown protocol)
  context...:
   /usr/share/racket/collects/openssl/mzssl.rkt:1401:8: loop
   /usr/share/racket/collects/openssl/..:261:28
   /home/schu/projects/racket-irc/src/irc/main.rkt:35:0: irc-get-connection9
   /home/schu/projects/racket-irc/src/irc/main.rkt:96:0: irc-connect21
   /home/schu/projects/racket-irc/src/irc/examples/echobot.rkt: [running body]
schuster commented 8 years ago

P.S. Sorry this took so long to get to - I never had a chance to look into it befor the holidays, and I only recently returned.

winny- commented 8 years ago

No worries, I'll take a look when I have a free moment this week.

winny- commented 8 years ago

@schuster I was able to test successfully after modifying the echobot.rkt file to use a port that permits SSL -- that's usually 7000, 6697, or the like. Both work for chat.freenode.net.

It looks like my changes broke the examples, so I'll push a commit fix to this branch momentarily.

This raises an interesting follow up pull request, this may need another way to provide a ssl context or otherwise options about how to verify the server's ssl/tls key.

schuster commented 8 years ago

Oh, of course, I should have realized I would need to change the port number.

As far as supplying a context, what do you think about this: the SSL option takes in the set of options that ssl-connect takes for client-protocol, plus #f for no SSL, and #t is a synonym for auto? Seems like it does the right thing in all cases.

winny- commented 8 years ago

That seems like the best way to approach the issue. I can look into making those changes later this week.

winny- commented 8 years ago

Hi! I managed to implement the said suggested changes. Please let me know if I should reformat my code. I'm not very experienced with making Racket look good. :)

schuster commented 8 years ago

I'm good with the formatting as-is. As small as this project is, I'm not sweating that too much.

Looks great, thanks for implementing this and going through a few PR cycles with it.

winny- commented 8 years ago

No problem, happy to see it work :shipit: