Re: [Qbittorrent-devel] HTTPS support

[Chris <dc...@gm...>]

Hi,

I have just made the following fixes to HTTPS support (for v2.8.0):
- Ask for Ssl Certificate / key only once. You connected the signal to a
slot called on_buttonName_clicked(). Such slot name is automatically
connected to the clicked signal already. Thus you were connected twice and
it would ask for the certificate (or the key) twice. How come you did not
see that?
- Simplify HTTPS settings: do not display the key/certificate content, just
an icon saying if they were set or not
- Add .crt / .key extension filter to file selection dialog
- Add Ssl key/certificate validation to program preferences
- Add HTTPS settings to Web UI

Kr,
Chris.

On Mon, May 16, 2011 at 9:58 PM, Ishan Arora <ish...@gm...> wrote:

> Just made the commit. Sorry for the delay. Got busy with work for a
> couple of days and then met with an accident. Now coding on the
> wall-mounted TV, lying on the bed.
>
> PS: does not update the preferences in Web UI.
>
> Regards,
> Ishan Arora
>
> Software Engineer
> System Software, Server Access Virtualization
> Cisco Systems
>
> http://www.ishanarora.com
>
>
>
>
> On Sun, May 8, 2011 at 10:34 PM, Chris <dc...@gm...> wrote:
> > Hi,
> >
> > - In HttpServer::disableHttps(), you should probably clear
> > m_certificate and m_key to make sure we don't waste memory.
> > - Please add
> http://httpd.apache.org/docs/2.1/ssl/ssl_faq.html#aboutcerts
> > link to program preferences to help users generate the cert/key.
> >
> > Besides that, it looks good. I can probably take care of updating the
> > Web UI. I already need to update it for other new settings before the
> > release anyway.
> >
> > Kr,
> > Chris.
> >
> > On Sun, May 8, 2011 at 2:31 PM, Ishan Arora <ish...@gm...>
> wrote:
> >> Hi,
> >>
> >> Please find attached a patch addressing your remarks in your last email.
> >>
> >>> - Some of the platforms we support don't have SSL support in Qt. It is
> >>> also possible for people to recompile Qt and explicitly disable Qt
> >>> there.
> >>> We would still like qBittorrent to compile on those systems.
> >>> Therefore, you need to wrap your new code into "#ifndef
> >>> QT_NO_OPENSSL". Do not forget to hide HTTPS settings in the UI when
> >>> QT_NO_OPENSSL is defined.
> >> Done in the patch
> >>
> >>> - Do not forget to pass method parameters by const references (not by
> >>> value). Implicit memory sharing in Qt helps but still :) I already
> >>> made the fix in preferences.h but I did not check elsewhere.
> >> Done in the patch
> >>
> >>> - In HttpServer::incomingConnection(): QSslSocket inhehits QTcpSocket.
> >>> As a consequence, you don't need to have two different socket. Just
> >>> use one QSslSocket (if QT_NO_OPENSSL is not defined) or a QTcpSocket
> >>> (if QT_NO_OPENSSL is defined). Then start the encryption only if
> >>> m_https is true.
> >> Done. I feel stupid for not reading QSslSocket class description before.
> :)
> >>
> >>> - You forgot to add code in QBtSession::initWebUi() to check if the
> >>> HTTPS state change (in order to update the HTTPServer accordingly). As
> >>> a consequence, the user has to restart qBittorrent to take the change
> >>> into consideration (or completely disable then reenable the Web UI).
> >>> Both as not acceptable.
> >> Done.
> >>
> >>> - In program settings (UI), I believe we should provide a link to a
> >>> documentation that explains how to generate SSL certificates / keys
> >>> (on all platforms). This is not an easy task for most users, let's
> >>> make their life easier.
> >> Go ahead. I am no expert on this, but I found
> >> http://httpd.apache.org/docs/2.1/ssl/ssl_faq.html#aboutcerts very
> >> helpful.
> >>
> >>> - The settings dialog in the Web UI should probably be updated to add
> >>> the new HTTPs settings there.
> >> I have completely lost touch with the WebUI code. Ironic? Could
> >> someone else do this? Thanks.
> >>
> >> Regards,
> >> Ishan Arora
> >>
> >> Software Engineer
> >> System Software, Server Access Virtualization
> >> Cisco Systems
> >>
> >> http://www.ishanarora.com
> >>
> >
>