social icon indicating copy to clipboard operation
social copied to clipboard
verified publisher icon nextcloud

fix: Handle when required `resource` parameter is missing or empty

Open joshtrichards opened this issue 1 year ago • 6 comments

  • Resolves: #1888
  • Target version: master

Summary

resource is a required parameter. Avoid a 500 error when it's missing or empty. Instead generate a proper 400 bad request response.

Refs:

  • https://datatracker.ietf.org/doc/html/rfc7033#section-4.2

P.S. Semi-related: Our default setup checks in server aren't compatible with social being enabled since they only check for 200 or 404 (neither of which are applicable when social is enabled and when querying /.well-known/webfinger without any parameterst):

https://github.com/nextcloud/server/blob/16812837157395c078a9689cd51530a6382e17d2/apps/settings/lib/SetupChecks/WellKnownUrls.php#L48

This PR doesn't impact those either way directly (since the prior 500 nor the 400 added via this PR would have ever passed). I'll try to take a closer look at those over in server. EDIT: nextcloud/server#49440

joshtrichards avatar Nov 21 '24 19:11 joshtrichards

Social    Run #985

Run Properties:  status check errored Errored #985  •  git commit 4c35775a1f: fix: Handle when required `resource` parameter is missing or empty
Project Social
Branch Review fix-missing-resource-issue-1888
Run status status check errored Errored #985
Run duration 01m 07s
Commit git commit 4c35775a1f: fix: Handle when required `resource` parameter is missing or empty
Committer Josh
View all properties for this run ↗︎
Test results
Tests that failed  Failures 2
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 0
View all changes introduced in this branch ↗︎

cypress[bot] avatar Nov 21 '24 19:11 cypress[bot]

This did not help for me, the error I now get is:

[index] Error: parse_str(): Argument #1 ($string) must be of type string, null given in file '/var/www/html/custom_apps/social/lib/WellKnown/WebfingerHandler.php' line 228
	GET /index.php/.well-known/webfinger
	from [...] by -- at 21 Nov 2024, 21:03:53

paulvt avatar Nov 21 '24 20:11 paulvt

@paulvt Oops. Try now. :)

joshtrichards avatar Nov 21 '24 20:11 joshtrichards

Unfortunately still the same. And I see the static-psalm-analysis GitHub Action says that strings cannot be null too :wink:

paulvt avatar Nov 21 '24 20:11 paulvt

Though I don't see how it should ever end up truly null. It should be ''. Even psalm agrees...

joshtrichards avatar Nov 21 '24 20:11 joshtrichards

This did not help for me, the error I now get is:

[index] Error: parse_str(): Argument #1 ($string) must be of type string, null given in file '/var/www/html/custom_apps/social/lib/WellKnown/WebfingerHandler.php' line 228
	GET /index.php/.well-known/webfinger
	from [...] by -- at 21 Nov 2024, 21:03:53

Same for me.

oculos avatar Nov 21 '24 20:11 oculos