Page MenuHomePhabricator

[keyserver] Remove extraneous verifyClientSupported calls
ClosedPublic

Authored by ashoat on Oct 24 2023, 12:13 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 15, 3:54 PM
Unknown Object (File)
Sat, Apr 13, 3:54 PM
Unknown Object (File)
Thu, Apr 11, 4:02 PM
Unknown Object (File)
Sun, Apr 7, 8:57 PM
Unknown Object (File)
Sun, Apr 7, 8:57 PM
Unknown Object (File)
Sun, Apr 7, 8:56 PM
Unknown Object (File)
Sun, Apr 7, 8:45 PM
Unknown Object (File)
Sat, Apr 6, 4:13 AM
Subscribers

Details

Summary

In the past, we had calls to validateInput in each responder, which in turn calls verifyClientSupported. When the input did not need validation because there was no input, we would instead add direct calls to verifyClientSupported.

However, now validateInput is called implicitly before each responder. As such, verifyClientSupported is also called implicitly, and we don't need any explicit calls to it in responders.

Test Plan

Careful reading of the code. I think this change is relatively safe since I'm just removing a call that already occurs elsewhere

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

keyserver/src/responders/keys-responders.js
69 ↗(On Diff #32353)

I first realized why this call was here from investigating @marcin's diff history for D7586. I could see there where await validateInput(viewer, null, null) was replaced with await verifyClientSupported(viewer)

keyserver/src/responders/user-responders.js
179–181 ↗(On Diff #32353)

This responder does nothing anyways...

203–205 ↗(On Diff #32353)

Honestly not even sure we should reject logouts from old clients, but it doesn't matter since the app immediately logs the user out (it doesn't wait for the response)

This revision is now accepted and ready to land.Oct 26 2023, 6:17 AM