Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Beta API: POST /api/apps returns 200 OK on invalid input JSON #3384

Open
2 tasks done
doxxx opened this issue Nov 9, 2024 · 10 comments · May be fixed by #3385
Open
2 tasks done

Beta API: POST /api/apps returns 200 OK on invalid input JSON #3384

doxxx opened this issue Nov 9, 2024 · 10 comments · May be fixed by #3385
Assignees
@ReenigneArcher
Labels
documentation Improvements or additions to documentation

Comments

@doxxx
Copy link

doxxx commented Nov 9, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Is your issue described in the documentation?

  • I have read the documentation

Is your issue present in the latest beta/pre-release?

I'm too lazy to test

Describe the Bug

When using the Beta API, a POST request to /api/apps with invalid input JSON will return a response of 200 OK, with a JSON document that describes the error. It should respond with 400 Bad Request.

Expected Behavior

Invalid POST request to /api/apps should return 400 Bad Request

Additional Context

No response

Host Operating System

Linux

Operating System Version

Linux aizen 6.11.6-zen1-1-zen #1 ZEN SMP PREEMPT_DYNAMIC Fri, 01 Nov 2024 03:30:35 +0000 x86_64 GNU/Linux

Architecture

amd64/x86_64

Sunshine commit or version

0.23.1

Package

Linux - AUR (Third Party)

GPU Type

n/a

GPU Model

n/a

GPU Driver/Mesa Version

n/a

Capture Method

None

Config

No response

Apps

No response

Relevant log output

n/a
@ReenigneArcher
Copy link
Member

with invalid input JSON

Provide more context please. What is the invalid data you provided to the API?

@doxxx
Copy link
Author

doxxx commented Nov 9, 2024

Right now, anything. I can't get it to accept any JSON, even if I follow the doc example exactly. For example:

{
  "name": "Test App",
  "output": "",
  "cmd": "/path/to/test/app",
  "exclude-global-prep-cmd": false,
  "elevated": false,
  "auto-detach": true,
  "wait-all": true,
  "exit-timeout": 5,
  "prep-cmd": [
  ],
  "detached": [
  ],
  "image-path": "",
}

@doxxx
Copy link
Author

doxxx commented Nov 9, 2024

Although the bug is not about the invalid data, it's the fact that the API returns 200 OK when it actually is rejecting the input.

@ReenigneArcher
Copy link
Member

ReenigneArcher commented Nov 9, 2024 •
edited
Loading

Can you share the full response from the server?

Edit: and logs.

@doxxx
Copy link
Author

doxxx commented Nov 9, 2024 

curl -v -k --request POST   --url https://localhost:47990/api/apps   --header 'authorization: Basic c3Vuc2hpbmU6c3Vuc2hpbmU='   --header 'content-type: application/json'   --header 'user-agen
t: vscode-restclient'   --data '{"name": "Test App","output": "","cmd": "/path/to/test/app","exclude-global-prep-cmd": false,"elevated": false,"auto-detach": true,"wait-all": true,"exit-timeout": 
5,"prep-cmd": [],"detached": [],"image-path": ""}'
Note: Unnecessary use of -X or --request, POST is already inferred.
* Host localhost:47990 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:47990...
* connect to ::1 port 47990 from ::1 port 42092 failed: Connection refused
*   Trying 127.0.0.1:47990...
* ALPN: curl offers h2,http/1.1
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.3 (IN), TLS change cipher, Change cipher spec (1):
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
* TLSv1.3 (IN), TLS handshake, Certificate (11):
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
* TLSv1.3 (IN), TLS handshake, Finished (20):
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.3 (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / TLS_AES_256_GCM_SHA384 / x25519 / RSASSA-PSS
* ALPN: server did not agree on a protocol. Uses default.
* Server certificate:
*  subject: CN=Sunshine Gamestream Host
*  start date: Oct 16 21:56:52 2024 GMT
*  expire date: Oct 11 21:56:52 2044 GMT
*  issuer: CN=Sunshine Gamestream Host
*  SSL certificate verify result: self-signed certificate (18), continuing anyway.
*   Certificate level 0: Public key type RSA (2048/112 Bits/secBits), signed using sha256WithRSAEncryption
* Connected to localhost (127.0.0.1) port 47990
* using HTTP/1.x
> POST /api/apps HTTP/1.1
> Host: localhost:47990
> Accept: */*
> authorization: Basic c3Vuc2hpbmU6c3Vuc2hpbmU=
> content-type: application/json
> user-agent: vscode-restclient
> Content-Length: 213
> 
* upload completely sent off: 213 bytes
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
< HTTP/1.1 200 OK
< Content-Length: 61
< 
{
    "status": "false",
    "error": "Invalid Input JSON"
}
* Connection #0 to host localhost left intact

@doxxx
Copy link
Author

doxxx commented Nov 9, 2024

Ahhhh... so initially it was failing because of a trailing comma. Now it's failing because of a missing "index" field.

@doxxx
Copy link
Author

doxxx commented Nov 9, 2024

Logs:

[2024:11:09:17:54:51]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:17:54:51]: Warning: SaveApp: No such node (prep-cmd)
[2024:11:09:17:55:08]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:17:55:08]: Warning: SaveApp: No such node (index)
[2024:11:09:17:56:24]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:17:56:24]: Warning: SaveApp: <unspecified file>(16): expected key string
[2024:11:09:17:56:29]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:17:56:29]: Warning: SaveApp: <unspecified file>(16): expected key string
[2024:11:09:17:57:52]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:18:02:46]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:18:02:46]: Warning: SaveApp: <unspecified file>(15): expected key string
[2024:11:09:18:02:54]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:18:02:54]: Warning: SaveApp: <unspecified file>(16): expected key string
[2024:11:09:18:05:17]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:18:05:17]: Warning: SaveApp: <unspecified file>(1): expected digits after -
[2024:11:09:18:05:28]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:18:05:28]: Warning: SaveApp: <unspecified file>(1): expected value
[2024:11:09:18:07:10]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:18:07:10]: Warning: SaveApp: <unspecified file>(15): expected key string
[2024:11:09:18:10:37]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:18:10:37]: Warning: SaveApp: <unspecified file>(15): expected key string
[2024:11:09:18:11:32]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:18:11:32]: Warning: SaveApp: No such node (index)
[2024:11:09:18:11:56]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:18:11:56]: Warning: SaveApp: No such node (index)
[2024:11:09:18:12:36]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:18:12:36]: Warning: SaveApp: No such node (index)
[2024:11:09:18:15:19]: Info: /home/gordon/.config/sunshine/apps.json
[2024:11:09:18:15:19]: Warning: SaveApp: No such node (index)

@ReenigneArcher
Copy link
Member

ReenigneArcher commented Nov 9, 2024 •
edited
Loading

Thanks for that extra info.

Yea, trailing commas are not valid JSON. I'll update the docs on those.

I thought index was no longer required. I'll also look into that.

Finally, we certainly shouldn't return a 200 when there's an error.

@ReenigneArcher ReenigneArcher added the documentation Improvements or additions to documentation label Nov 9, 2024
@ReenigneArcher ReenigneArcher self-assigned this Nov 9, 2024
@ReenigneArcher ReenigneArcher linked a pull request Nov 10, 2024 that will close this issue
Draft
10 tasks
@doxxx
Copy link
Author

doxxx commented Nov 12, 2024

Regarding index, I think it is still necessary so that you can edit an app name's name without accidentally creating a new app.

@ReenigneArcher
Copy link
Member

Yes, that's correct. I discovered that after reviewing the code in more detail, and will improve the documentation around that in the PR linked above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ReenigneArcher ReenigneArcher

Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(confighttp): do not return 200 on errors LizardByte/Sunshine
2 participants
@doxxx @ReenigneArcher