195247 2019-03-02 10:32:38 -0800 [SOUP] Cleanups in SoupNetworkSession 2019-03-03 09:51:44 -0800 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build PC Linux RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=195257 P2 Normal --- 1 mcatanzaro mcatanzaro berto bugs-noreply cgarcia dbates ews-watchlist gustavo mcatanzaro oldest_to_newest 1511828 0 mcatanzaro 2019-03-02 10:32:38 -0800 clientCertificates actually contains server certificates, not client certificates. Client certificate support is still on my TODO. 1511829 1 363424 mcatanzaro 2019-03-02 10:38:28 -0800 Created attachment 363424 Patch 1511862 2 363424 dbates 2019-03-02 15:12:46 -0800 Comment on attachment 363424 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363424&action=review > Source/WebCore/platform/network/soup/SoupNetworkSession.cpp:101 > +static HashMap<String, HostTLSCertificateSet, ASCIICaseInsensitiveHash>& allowedCertificates() Ok as-is. Just a thought, define a type alias for the HashMap to avoid repetition below. > Source/WebCore/platform/network/soup/SoupNetworkSession.cpp:287 > + auto it = allowedCertificates().find(requestURL.host().toString()); Ok as-is, but can be made a bit more efficient by using toStringWithoutCopying(). Even better, as I type this I could swear we solved this issue with template magic and made it so you could pass StringView to HashMap::find(). Can you check? I think we call this magic a HashTranslator or adapter and there is a template overload of HashMap::find(). And if we don’t have such magic because we are missing the HashTranslator for StringView then please either add it OR file a bug so that we can add it. 1511945 3 mcatanzaro 2019-03-03 09:50:36 -0800 (In reply to Daniel Bates from comment #2) > Ok as-is. Just a thought, define a type alias for the HashMap to avoid > repetition below. Yeah, OK. > > Source/WebCore/platform/network/soup/SoupNetworkSession.cpp:287 > > + auto it = allowedCertificates().find(requestURL.host().toString()); > > Ok as-is, but can be made a bit more efficient by using > toStringWithoutCopying(). OK. > Even better, as I type this I could swear we > solved this issue with template magic and made it so you could pass > StringView to HashMap::find(). Can you check? I think we call this magic a > HashTranslator or adapter and there is a template overload of > HashMap::find(). And if we don’t have such magic because we are missing the > HashTranslator for StringView then please either add it OR file a bug so > that we can add it. Doesn't work, reported bug #195257. 1511946 4 mcatanzaro 2019-03-03 09:51:44 -0800 Committed r242328: <https://trac.webkit.org/changeset/242328> 363424 2019-03-02 10:38:28 -0800 2019-03-02 15:12:46 -0800 Patch bug-195247-20190302123827.patch text/plain 2850 mcatanzaro U3VidmVyc2lvbiBSZXZpc2lvbjogMjQyMjY0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggOWYwZWQxMTkxYmIwNjRk ZmU2NmRmNzNkYmI2M2VkMmE3OTZmZTVjOS4uZmNkZDdkZWJmZThjZGZhZTc1OWUyYTM3MDRlZGUy ODA3YjhjYmQwYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDE5LTAzLTAyICBNaWNo YWVsIENhdGFuemFybyAgPG1jYXRhbnphcm9AaWdhbGlhLmNvbT4KKworICAgICAgICBbU09VUF0g TWlzbmFtZWQgZnVuY3Rpb24gaW4gU291cE5ldHdvcmtTZXNzaW9uCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTUyNDcKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGlzIGp1c3QgcmVuYW1lcyBjbGllbnRD ZXJ0aWZpY2F0ZXMgdG8gYWxsb3dlZENlcnRpZmljYXRlcywgc2luY2UgdGhleSBhcmUgbm90IGNs aWVudAorICAgICAgICBjZXJ0aWZpY2F0ZXMgYXQgYWxsLCBidXQgc2VydmVyIGNlcnRpZmljYXRl cy4KKworICAgICAgICAqIHBsYXRmb3JtL25ldHdvcmsvc291cC9Tb3VwTmV0d29ya1Nlc3Npb24u Y3BwOgorICAgICAgICAoV2ViQ29yZTo6YWxsb3dlZENlcnRpZmljYXRlcyk6CisgICAgICAgIChX ZWJDb3JlOjpTb3VwTmV0d29ya1Nlc3Npb246OmNoZWNrVExTRXJyb3JzKToKKyAgICAgICAgKFdl YkNvcmU6OlNvdXBOZXR3b3JrU2Vzc2lvbjo6YWxsb3dTcGVjaWZpY0hUVFBTQ2VydGlmaWNhdGVG b3JIb3N0KToKKyAgICAgICAgKFdlYkNvcmU6OmNsaWVudENlcnRpZmljYXRlcyk6IERlbGV0ZWQu CisKIDIwMTktMDMtMDEgIEp1c3RpbiBGYW4gIDxqdXN0aW5fZmFuQGFwcGxlLmNvbT4KIAogICAg ICAgICBbV2ViIEdQVV0gMzItYml0IGJ1aWxkcyBicm9rZW4gYnkgYXR0ZW1wdCB0byBkaXNhYmxl IFdlYkdQVSBvbiAzMi1iaXQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL25l dHdvcmsvc291cC9Tb3VwTmV0d29ya1Nlc3Npb24uY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vbmV0d29yay9zb3VwL1NvdXBOZXR3b3JrU2Vzc2lvbi5jcHAKaW5kZXggYjk2Nzc5NTk4MGVl OGZhNGM5OTUwMGZmZGYzNGI0MjAyZWIyYTJkYi4uOGNkZGIzNjU0MjE0MDFiZTdkMzYxMjk1MjQw ZjhjNGZhNmEzNWU2YyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29y ay9zb3VwL1NvdXBOZXR3b3JrU2Vzc2lvbi5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vbmV0d29yay9zb3VwL1NvdXBOZXR3b3JrU2Vzc2lvbi5jcHAKQEAgLTk4LDcgKzk4LDcgQEAg cHJpdmF0ZToKICAgICBIYXNoU2V0PFN0cmluZz4gbV9jZXJ0aWZpY2F0ZXM7CiB9OwogCi1zdGF0 aWMgSGFzaE1hcDxTdHJpbmcsIEhvc3RUTFNDZXJ0aWZpY2F0ZVNldCwgQVNDSUlDYXNlSW5zZW5z aXRpdmVIYXNoPiYgY2xpZW50Q2VydGlmaWNhdGVzKCkKK3N0YXRpYyBIYXNoTWFwPFN0cmluZywg SG9zdFRMU0NlcnRpZmljYXRlU2V0LCBBU0NJSUNhc2VJbnNlbnNpdGl2ZUhhc2g+JiBhbGxvd2Vk Q2VydGlmaWNhdGVzKCkKIHsKICAgICBzdGF0aWMgTmV2ZXJEZXN0cm95ZWQ8SGFzaE1hcDxTdHJp bmcsIEhvc3RUTFNDZXJ0aWZpY2F0ZVNldCwgQVNDSUlDYXNlSW5zZW5zaXRpdmVIYXNoPj4gY2Vy dGlmaWNhdGVzOwogICAgIHJldHVybiBjZXJ0aWZpY2F0ZXM7CkBAIC0yODQsOCArMjg0LDggQEAg T3B0aW9uYWw8UmVzb3VyY2VFcnJvcj4gU291cE5ldHdvcmtTZXNzaW9uOjpjaGVja1RMU0Vycm9y cyhjb25zdCBVUkwmIHJlcXVlc3RVUkwKICAgICBpZiAoIXRsc0Vycm9ycykKICAgICAgICAgcmV0 dXJuIFdURjo6bnVsbG9wdDsKIAotICAgIGF1dG8gaXQgPSBjbGllbnRDZXJ0aWZpY2F0ZXMoKS5m aW5kKHJlcXVlc3RVUkwuaG9zdCgpLnRvU3RyaW5nKCkpOwotICAgIGlmIChpdCAhPSBjbGllbnRD ZXJ0aWZpY2F0ZXMoKS5lbmQoKSAmJiBpdC0+dmFsdWUuY29udGFpbnMoY2VydGlmaWNhdGUpKQor ICAgIGF1dG8gaXQgPSBhbGxvd2VkQ2VydGlmaWNhdGVzKCkuZmluZChyZXF1ZXN0VVJMLmhvc3Qo KS50b1N0cmluZygpKTsKKyAgICBpZiAoaXQgIT0gYWxsb3dlZENlcnRpZmljYXRlcygpLmVuZCgp ICYmIGl0LT52YWx1ZS5jb250YWlucyhjZXJ0aWZpY2F0ZSkpCiAgICAgICAgIHJldHVybiBXVEY6 Om51bGxvcHQ7CiAKICAgICByZXR1cm4gUmVzb3VyY2VFcnJvcjo6dGxzRXJyb3IocmVxdWVzdFVS TCwgdGxzRXJyb3JzLCBjZXJ0aWZpY2F0ZSk7CkBAIC0yOTMsNyArMjkzLDcgQEAgT3B0aW9uYWw8 UmVzb3VyY2VFcnJvcj4gU291cE5ldHdvcmtTZXNzaW9uOjpjaGVja1RMU0Vycm9ycyhjb25zdCBV UkwmIHJlcXVlc3RVUkwKIAogdm9pZCBTb3VwTmV0d29ya1Nlc3Npb246OmFsbG93U3BlY2lmaWNI VFRQU0NlcnRpZmljYXRlRm9ySG9zdChjb25zdCBDZXJ0aWZpY2F0ZUluZm8mIGNlcnRpZmljYXRl SW5mbywgY29uc3QgU3RyaW5nJiBob3N0KQogewotICAgIGNsaWVudENlcnRpZmljYXRlcygpLmFk ZChob3N0LCBIb3N0VExTQ2VydGlmaWNhdGVTZXQoKSkuaXRlcmF0b3ItPnZhbHVlLmFkZChjZXJ0 aWZpY2F0ZUluZm8uY2VydGlmaWNhdGUoKSk7CisgICAgYWxsb3dlZENlcnRpZmljYXRlcygpLmFk ZChob3N0LCBIb3N0VExTQ2VydGlmaWNhdGVTZXQoKSkuaXRlcmF0b3ItPnZhbHVlLmFkZChjZXJ0 aWZpY2F0ZUluZm8uY2VydGlmaWNhdGUoKSk7CiB9CiAKIH0gLy8gbmFtZXNwYWNlIFdlYkNvcmUK