Bug 50344 - wss (websockets ssl) support for gtk via new gio TLS support
Summary: wss (websockets ssl) support for gtk via new gio TLS support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 50675
Blocks:
  Show dependency treegraph
 
Reported: 2010-12-01 12:17 PST by Dan Winship
Modified: 2011-01-31 11:44 PST (History)
3 users (show)

See Also:


Attachments
Update SocketStreamHandleSoup to do wss:// (8.15 KB, patch)
2010-12-01 12:17 PST, Dan Winship
no flags Details | Formatted Diff | Diff
fix style nits (8.17 KB, patch)
2010-12-02 07:48 PST, Dan Winship
no flags Details | Formatted Diff | Diff
updated patch (7.74 KB, patch)
2011-01-29 16:16 PST, Dan Winship
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dan Winship 2010-12-01 12:17:40 PST
Created attachment 75301 [details]
Update SocketStreamHandleSoup to do wss://

Attached patch uses GTlsConnection to make wss:// work. You can test it against http://websockets.org/echo.html.

(The GPollableOutputStream stuff is basically an abstraction of the previous GSocket-specific API, that works on both GSocketConnection and GTlsConnection.)

Notably missing from this patch is a change to configure.in to require glib >= 2.27.4 since I wasn't sure how you want to do that. (Currently the support for gsettings is just conditional on the glib version... so we could do the same thing here, and continue using the old ws://-only code with older glibs...)
Comment 1 WebKit Review Bot 2010-12-01 12:20:23 PST
Attachment 75301 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'JavaScriptCore/ChangeLog', u'JavaScriptCore/wtf/gobject/GTypedefs.h', u'WebCore/ChangeLog', u'WebCore/platform/network/soup/SocketStreamHandle.h', u'WebCore/platform/network/soup/SocketStreamHandleSoup.cpp']" exit_code: 1
WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
WebCore/platform/network/soup/SocketStreamHandleSoup.cpp:90:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/network/soup/SocketStreamHandleSoup.cpp:165:  Extra space for operator !   [whitespace/operators] [4]
Total errors found: 3 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 2 Martin Robinson 2010-12-01 17:26:26 PST
Comment on attachment 75301 [details]
Update SocketStreamHandleSoup to do wss://

View in context: https://bugs.webkit.org/attachment.cgi?id=75301&action=review

Awesome! The addition of SSL socket support in GIO is great news. Will it need to be behind some version guards though, so that we will still work properly with older GIO versions?

> WebCore/platform/network/soup/SocketStreamHandleSoup.cpp:90
> +	g_socket_client_set_tls(socketClient.get(), TRUE);

Indentation is slightly off here.

> WebCore/platform/network/soup/SocketStreamHandleSoup.cpp:165
> +    if (error && ! g_error_matches(error.get(), G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {

One extra space between ! and g_error_matches.

> WebCore/platform/network/soup/SocketStreamHandleSoup.cpp:274
> -    // G_IO_HUP and G_IO_ERR are are always active. See:
> -    // http://library.gnome.org/devel/gio/stable/GSocket.html#g-socket-create-source
> -    if (condition & G_IO_HUP) {
> -        handle->close();
> -        return FALSE;
> -    }
> -    if (condition & G_IO_ERR) {
> -        handle->client()->didFail(handle, SocketStreamError(0)); // FIXME: Provide a sensible error.
> -        return FALSE;
> -    }
> -    if (condition & G_IO_OUT)
> -        handle->writeReady();
> +    handle->writeReady();

Are connection erros (such as HUP) still handled properly?
Comment 3 Dan Winship 2010-12-02 07:46:09 PST
(In reply to comment #2)
> Awesome! The addition of SSL socket support in GIO is great news. Will it need to be behind some version guards though, so that we will still work properly with older GIO versions?

Yes. (Mentioned in comment 0. I didn't deal with it in the patch because I wasn't sure how we wanted to deal with it.)

> Are connection errors (such as HUP) still handled properly?

Yes; when it eventually calls g_pollable_output_stream_write_nonblocking() again, the error will be returned at that point.
Comment 4 Dan Winship 2010-12-02 07:48:09 PST
Created attachment 75378 [details]
fix style nits
Comment 5 Dan Winship 2010-12-08 04:37:45 PST
Comment on attachment 75378 [details]
fix style nits

versioning issues will be taken care of by bug 50675
Comment 6 WebKit Review Bot 2010-12-10 19:44:02 PST
Attachment 75378 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/6970047
Comment 7 Dan Winship 2011-01-29 16:16:40 PST
Created attachment 80580 [details]
updated patch

updated to top of tree, and ready to go now that the required glib version has been updated
Comment 8 Gustavo Noronha (kov) 2011-01-31 10:31:29 PST
Comment on attachment 80580 [details]
updated patch

Looks good to me. As discussed with danw on IRC I'll add the bug URL and title, fix a nit (remove a line break) and land.
Comment 9 Gustavo Noronha (kov) 2011-01-31 11:37:32 PST
Committed r77148: <http://trac.webkit.org/changeset/77148>
Comment 10 Gustavo Noronha (kov) 2011-01-31 11:44:34 PST
Comment on attachment 80580 [details]
updated patch

Landed!