Bug 230477 - Disable FTP
Summary: Disable FTP
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-09-20 06:01 PDT by Brady Eidson
Modified: 2021-09-22 10:59 PDT (History)
6 users (show)

See Also:


Attachments
Patch (which no changeling, still sorting that out) (25.17 KB, patch)
2021-09-20 06:07 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
v2 (24.69 KB, patch)
2021-09-20 06:15 PDT, Brady Eidson
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
v3 (25.00 KB, patch)
2021-09-20 07:17 PDT, Brady Eidson
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
v4 (24.86 KB, patch)
2021-09-20 08:22 PDT, Brady Eidson
ggaren: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
PFL for EWS (27.82 KB, patch)
2021-09-20 18:45 PDT, Brady Eidson
beidson: commit-queue-
Details | Formatted Diff | Diff
PFL for EWS #2 (28.62 KB, patch)
2021-09-22 09:52 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2021-09-20 06:01:48 PDT
Disable FTP

We'll do this as an on-by-default experimental feature

<rdar://81193860>
Comment 1 Brady Eidson 2021-09-20 06:07:19 PDT
Created attachment 438665 [details]
Patch (which no changeling, still sorting that out)
Comment 2 Brady Eidson 2021-09-20 06:07:46 PDT
(In reply to Brady Eidson from comment #1)
> Created attachment 438665 [details]
> Patch (which no changeling, still sorting that out)

ChangeLog <--- DYAC
Comment 3 Brady Eidson 2021-09-20 06:15:24 PDT
Created attachment 438666 [details]
v2
Comment 4 Brady Eidson 2021-09-20 06:20:53 PDT
Comment on attachment 438666 [details]
v2

Marking for review. Changelog on its way later
Comment 5 Brady Eidson 2021-09-20 07:17:29 PDT
Created attachment 438674 [details]
v3
Comment 6 Brady Eidson 2021-09-20 08:22:02 PDT
Created attachment 438680 [details]
v4
Comment 7 Geoffrey Garen 2021-09-20 11:51:32 PDT
Comment on attachment 438680 [details]
v4

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

r=me

Seems fine. Please rename the setting before landing.

EWS failure seems pre-existing.

> Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml:424
> +FTPDisabled:

I prefer settings to be nouns or affirmative phrases. That way, we can avoid the double negative of talking about when "ftp disabling is disabled". Let's call this just FTP or FTPEnabled or FTPSupport.
Comment 8 Brady Eidson 2021-09-20 14:26:46 PDT
(In reply to Geoffrey Garen from comment #7)
> Comment on attachment 438680 [details]
> v4
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=438680&action=review
> 
> r=me
> 
> Seems fine. Please rename the setting before landing.
> 
> EWS failure seems pre-existing.
> 
> > Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml:424
> > +FTPDisabled:
> 
> I prefer settings to be nouns or affirmative phrases. That way, we can avoid
> the double negative of talking about when "ftp disabling is disabled". Let's
> call this just FTP or FTPEnabled or FTPSupport.

The reason we made it a negative (and there's precedent for this) is because Experimental Runtime Features are supposed to start off and then become enabled.

I'll ping you on Slack to followup.
Comment 9 Brady Eidson 2021-09-20 18:45:13 PDT
Created attachment 438768 [details]
PFL for EWS
Comment 10 Brady Eidson 2021-09-20 20:06:44 PDT
Cq+'ed too early - Wanna make sure the API bots are happy
Comment 11 EWS 2021-09-21 10:12:53 PDT
Found 1 new test failure: imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird.html
Comment 12 Brady Eidson 2021-09-21 10:56:12 PDT
(In reply to EWS from comment #11)
> Found 1 new test failure:
> imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/
> location-protocol-setter-non-broken-weird.html

This appears to be a legit error.

Locally I'd already moved on from this patch, it'll take some time to get back to it to explore.
Comment 13 Brady Eidson 2021-09-21 16:34:07 PDT
(In reply to Brady Eidson from comment #12)
> (In reply to EWS from comment #11)
> > Found 1 new test failure:
> > imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/
> > location-protocol-setter-non-broken-weird.html
> 
> This appears to be a legit error.
> 
> Locally I'd already moved on from this patch, it'll take some time to get
> back to it to explore.

Up and running and can reproduce.
Comment 14 Brady Eidson 2021-09-22 09:52:53 PDT
Created attachment 438959 [details]
PFL for EWS #2
Comment 15 EWS 2021-09-22 10:58:53 PDT
Committed r282881 (242010@main): <https://commits.webkit.org/242010@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 438959 [details].
Comment 16 Radar WebKit Bug Importer 2021-09-22 10:59:16 PDT
<rdar://problem/83408253>