192959 2018-12-20 16:01:51 -0800 Web Inspector: Charles Proxy errors opening har files exported from Safari (invalid startedDateTime) 2018-12-20 16:58:54 -0800 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck bburg commit-queue hi inspector-bugzilla-changes joepeck webkit-bug-importer oldest_to_newest 1490127 0 joepeck 2018-12-20 16:01:51 -0800 Charles Proxy errors opening har files exported from Safari (invalid startedDateTime) Steps to Reproduce: 1. Open apple.com 2. Open inspector after page load 3. Export a HAR from Network Tab (lacking resource load data, just listing existing resource) 4. Open HAR in Charles => ‘JsonParse: Invalid format: “”’ Note: • This is due to `"startedDateTime" = ""` in the HAR content • `startedDateTime` is non-optional in entries and pages: http://www.softwareishard.com/blog/har-12-spec/ Proposal: We should not allow exporting a HAR without full load data. So disallow HAR export when just opening the inspector for a page but allowing it on reload / navigation once we have main resource content. 1490129 1 joepeck 2018-12-20 16:02:01 -0800 <rdar://problem/46863411> 1490146 2 357893 joepeck 2018-12-20 16:11:42 -0800 Created attachment 357893 [PATCH] Proposed Fix 1490170 3 357893 bburg 2018-12-20 16:29:11 -0800 Comment on attachment 357893 [PATCH] Proposed Fix r=me Is there a way to test this? And, we probably should update documentation somewhere to explain this restriction. 1490172 4 357893 hi 2018-12-20 16:32:03 -0800 Comment on attachment 357893 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=357893&action=review > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1167 > + let mainResource = mainFrame.mainResource; Considering that the HAR is built based on the filtered resources in the table, is this check necessary? Do we allow an export without the main resource? > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1168 > + if (!mainResource) Aside: is it ever possible for the `mainFrame` to not have a `mainResource`? > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1174 > + if (!this._HARResources().length) NIT: I know that this isn't part of this patch, but shouldn't this be named `_harResouces`? 1490174 5 joepeck 2018-12-20 16:36:24 -0800 (In reply to Brian Burg from comment #3) > Comment on attachment 357893 [details] > [PATCH] Proposed Fix > > r=me > > Is there a way to test this? And, we probably should update documentation > somewhere to explain this restriction. Existing HAR tests passed. The rest is primarily a UI restriction. (In reply to Devin Rousso from comment #4) > Comment on attachment 357893 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=357893&action=review > > > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1167 > > + let mainResource = mainFrame.mainResource; > > Considering that the HAR is built based on the filtered resources in the > table, is this check necessary? Do we allow an export without the main > resource? We can export a subset, but the HAR itself will need top level info. > > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1168 > > + if (!mainResource) > > Aside: is it ever possible for the `mainFrame` to not have a `mainResource`? Probably not, but might as well check. I don't know about cases like ITMLKit. > > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1174 > > + if (!this._HARResources().length) > > NIT: I know that this isn't part of this patch, but shouldn't this be named > `_harResouces`? Either works. 1490189 6 357893 commit-queue 2018-12-20 16:58:53 -0800 Comment on attachment 357893 [PATCH] Proposed Fix Clearing flags on attachment: 357893 Committed r239478: <https://trac.webkit.org/changeset/239478> 1490190 7 commit-queue 2018-12-20 16:58:54 -0800 All reviewed patches have been landed. Closing bug. 357893 2018-12-20 16:11:42 -0800 2018-12-20 16:58:53 -0800 [PATCH] Proposed Fix har-1.patch text/plain 3990 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IGI4YjBhODRhODE5Li4xNmExNjM1MWM0NSAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjEgQEAKKzIwMTgtMTItMjAgIEpvc2Vw aCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKworICAgICAgICBXZWIgSW5zcGVjdG9y OiBDaGFybGVzIFByb3h5IGVycm9ycyBvcGVuaW5nIGhhciBmaWxlcyBleHBvcnRlZCBmcm9tIFNh ZmFyaSAoaW52YWxpZCBzdGFydGVkRGF0ZVRpbWUpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTI5NTkKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzQ2 ODYzNDExPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg ICogVXNlckludGVyZmFjZS9WaWV3cy9OZXR3b3JrVGFibGVDb250ZW50Vmlldy5qczoKKyAgICAg ICAgKFdJLk5ldHdvcmtUYWJsZUNvbnRlbnRWaWV3LnByb3RvdHlwZS50YWJsZUNlbGxDb250ZXh0 TWVudUNsaWNrZWQpOgorICAgICAgICAoV0kuTmV0d29ya1RhYmxlQ29udGVudFZpZXcucHJvdG90 eXBlLl91cGRhdGVFeHBvcnRCdXR0b24pOgorICAgICAgICAoV0kuTmV0d29ya1RhYmxlQ29udGVu dFZpZXcucHJvdG90eXBlLl9jYW5FeHBvcnRIQVIpOgorICAgICAgICBEaXNhbGxvdyBIQVIgZXhw b3J0IGlmIHdlIGRvbid0IGhhdmUgbG9hZCBkYXRhLCBzdWNoIGFzIHRoZSByZXF1ZXN0U2VudERh dGUgZm9yCisgICAgICAgIHRoZSBtYWluIHJlc291cmNlIG9yIHN1Yi1yZXNvdXJjZXMuCisKKyAg ICAgICAgKFdJLk5ldHdvcmtUYWJsZUNvbnRlbnRWaWV3LnByb3RvdHlwZS5fSEFSUmVzb3VyY2Vz KToKKyAgICAgICAgT25seSBleHBvcnQgcmVzb3VyY2VzIHRoYXQgaGF2ZSBsb2FkIGRhdGEuCisK IDIwMTgtMTItMjAgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KIAogICAg ICAgICBXZWIgSW5zcGVjdG9yOiBBdXRvZm9ybWF0IGRvZXNuJ3Qgd29yayBvbiBpY2xvdWQuY29t IChqYXZhc2NyaXB0LXBhY2tlZC5qcykKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JV SS9Vc2VySW50ZXJmYWNlL1ZpZXdzL05ldHdvcmtUYWJsZUNvbnRlbnRWaWV3LmpzIGIvU291cmNl L1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvTmV0d29ya1RhYmxlQ29udGVudFZp ZXcuanMKaW5kZXggNmFmMTliZjdkZGYuLjU3ZWM4YWYzZjEwIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9OZXR3b3JrVGFibGVDb250ZW50Vmll dy5qcworKysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9OZXR3 b3JrVGFibGVDb250ZW50Vmlldy5qcwpAQCAtMTQ3LDcgKzE0Nyw3IEBAIFdJLk5ldHdvcmtUYWJs ZUNvbnRlbnRWaWV3ID0gY2xhc3MgTmV0d29ya1RhYmxlQ29udGVudFZpZXcgZXh0ZW5kcyBXSS5D b250ZW50VmllCiAgICAgICAgIC8vIHRyaWdnZXJzIGEgTWFpblJlc291cmNlIGNoYW5nZSBhbmQg dGhlbiBhIE1haW5GcmFtZSBjaGFuZ2UuIFBhZ2UgVHJhbnNpdGlvbgogICAgICAgICAvLyB0cmln Z2VycyBhIE1haW5GcmFtZSBjaGFuZ2UgdGhlbiBhIE1haW5SZXNvdXJjZSBjaGFuZ2UuCiAgICAg ICAgIHRoaXMuX3RyYW5zaXRpb25pbmdQYWdlVGFyZ2V0ID0gZmFsc2U7Ci0gICAgICAgIAorCiAg ICAgICAgIFdJLm5vdGlmaWNhdGlvbnMuYWRkRXZlbnRMaXN0ZW5lcihXSS5Ob3RpZmljYXRpb24u VHJhbnNpdGlvblBhZ2VUYXJnZXQsIHRoaXMuX3RyYW5zaXRpb25QYWdlVGFyZ2V0LCB0aGlzKTsK ICAgICB9CiAKQEAgLTM2Myw3ICszNjMsNyBAQCBXSS5OZXR3b3JrVGFibGVDb250ZW50VmlldyA9 IGNsYXNzIE5ldHdvcmtUYWJsZUNvbnRlbnRWaWV3IGV4dGVuZHMgV0kuQ29udGVudFZpZQogICAg ICAgICBXSS5hcHBlbmRDb250ZXh0TWVudUl0ZW1zRm9yU291cmNlQ29kZShjb250ZXh0TWVudSwg ZW50cnkucmVzb3VyY2UpOwogCiAgICAgICAgIGNvbnRleHRNZW51LmFwcGVuZFNlcGFyYXRvcigp OwotICAgICAgICBjb250ZXh0TWVudS5hcHBlbmRJdGVtKFdJLlVJU3RyaW5nKCJFeHBvcnQgSEFS IiksICgpID0+IHsgdGhpcy5fZXhwb3J0SEFSKCk7IH0pOworICAgICAgICBjb250ZXh0TWVudS5h cHBlbmRJdGVtKFdJLlVJU3RyaW5nKCJFeHBvcnQgSEFSIiksICgpID0+IHsgdGhpcy5fZXhwb3J0 SEFSKCk7IH0sICF0aGlzLl9jYW5FeHBvcnRIQVIoKSk7CiAgICAgfQogCiAgICAgdGFibGVTaG91 bGRTZWxlY3RSb3codGFibGUsIGNlbGwsIGNvbHVtbiwgcm93SW5kZXgpCkBAIC0xMTU4LDEwICsx MTU4LDI4IEBAIFdJLk5ldHdvcmtUYWJsZUNvbnRlbnRWaWV3ID0gY2xhc3MgTmV0d29ya1RhYmxl Q29udGVudFZpZXcgZXh0ZW5kcyBXSS5Db250ZW50VmllCiAgICAgICAgIH0KICAgICB9CiAKKyAg ICBfY2FuRXhwb3J0SEFSKCkKKyAgICB7CisgICAgICAgIGxldCBtYWluRnJhbWUgPSBXSS5uZXR3 b3JrTWFuYWdlci5tYWluRnJhbWU7CisgICAgICAgIGlmICghbWFpbkZyYW1lKQorICAgICAgICAg ICAgcmV0dXJuIGZhbHNlOworCisgICAgICAgIGxldCBtYWluUmVzb3VyY2UgPSBtYWluRnJhbWUu bWFpblJlc291cmNlOworICAgICAgICBpZiAoIW1haW5SZXNvdXJjZSkKKyAgICAgICAgICAgIHJl dHVybiBmYWxzZTsKKworICAgICAgICBpZiAoIW1haW5SZXNvdXJjZS5yZXF1ZXN0U2VudERhdGUp CisgICAgICAgICAgICByZXR1cm4gZmFsc2U7CisKKyAgICAgICAgaWYgKCF0aGlzLl9IQVJSZXNv dXJjZXMoKS5sZW5ndGgpCisgICAgICAgICAgICByZXR1cm4gZmFsc2U7CisKKyAgICAgICAgcmV0 dXJuIHRydWU7CisgICAgfQorCiAgICAgX3VwZGF0ZUV4cG9ydEJ1dHRvbigpCiAgICAgewotICAg ICAgICBsZXQgZW5hYmxlZCA9IHRoaXMuX2ZpbHRlcmVkRW50cmllcy5sZW5ndGggPiAwOwotICAg ICAgICB0aGlzLl9oYXJFeHBvcnROYXZpZ2F0aW9uSXRlbS5lbmFibGVkID0gZW5hYmxlZDsKKyAg ICAgICAgdGhpcy5faGFyRXhwb3J0TmF2aWdhdGlvbkl0ZW0uZW5hYmxlZCA9IHRoaXMuX2NhbkV4 cG9ydEhBUigpOwogICAgIH0KIAogICAgIF9wcm9jZXNzUGVuZGluZ0VudHJpZXMoKQpAQCAtMTg3 MSw3ICsxODg5LDE1IEBAIFdJLk5ldHdvcmtUYWJsZUNvbnRlbnRWaWV3ID0gY2xhc3MgTmV0d29y a1RhYmxlQ29udGVudFZpZXcgZXh0ZW5kcyBXSS5Db250ZW50VmllCiAgICAgewogICAgICAgICBs ZXQgcmVzb3VyY2VzID0gdGhpcy5fZmlsdGVyZWRFbnRyaWVzLm1hcCgoeCkgPT4geC5yZXNvdXJj ZSk7CiAgICAgICAgIGNvbnN0IHN1cHBvcnRlZEhBUlNjaGVtZXMgPSBuZXcgU2V0KFsiaHR0cCIs ICJodHRwcyIsICJ3cyIsICJ3c3MiXSk7Ci0gICAgICAgIHJldHVybiByZXNvdXJjZXMuZmlsdGVy KChyZXNvdXJjZSkgPT4gcmVzb3VyY2UuZmluaXNoZWQgJiYgc3VwcG9ydGVkSEFSU2NoZW1lcy5o YXMocmVzb3VyY2UudXJsQ29tcG9uZW50cy5zY2hlbWUpKTsKKyAgICAgICAgcmV0dXJuIHJlc291 cmNlcy5maWx0ZXIoKHJlc291cmNlKSA9PiB7CisgICAgICAgICAgICBpZiAoIXJlc291cmNlLmZp bmlzaGVkKQorICAgICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICAgICAgICAgIGlmICgh cmVzb3VyY2UucmVxdWVzdFNlbnREYXRlKQorICAgICAgICAgICAgICAgIHJldHVybiBmYWxzZTsK KyAgICAgICAgICAgIGlmICghc3VwcG9ydGVkSEFSU2NoZW1lcy5oYXMocmVzb3VyY2UudXJsQ29t cG9uZW50cy5zY2hlbWUpKQorICAgICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICAgICAg ICAgIHJldHVybiB0cnVlOworICAgICAgICB9KTsKICAgICB9CiAKICAgICBfZXhwb3J0SEFSKCkK