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.

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

fix(backend): return HTTP 404 for any unknown api endpoint paths #10130

Merged
merged 2 commits into from
Feb 27, 2023

Conversation

saschanaz
Copy link
Member

@saschanaz saschanaz commented Feb 26, 2023

...PR template...

なにを

/api/下の不正なアドレスにアクセスしたらHTTP 404が出るようにしました

なんで

いままではHTTP 200とクライアントのHTMLページを出して混乱を招いていましたので

Fixes #9714

@github-actions github-actions bot added packages/backend Server side specific issue/PR 🧪Test labels Feb 26, 2023
@codecov
Copy link

codecov bot commented Feb 26, 2023

Codecov Report

Merging #10130 (f1a2eb8) into develop (81e6a21) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop   #10130      +/-   ##
===========================================
- Coverage    25.06%   25.05%   -0.01%     
===========================================
  Files          705      705              
  Lines        65151    65167      +16     
  Branches      2307     2307              
===========================================
  Hits         16329    16329              
- Misses       48822    48838      +16     
Impacted Files Coverage Δ
...ackages/backend/src/server/api/ApiServerService.ts 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@rinsuki
Copy link
Contributor

rinsuki commented Feb 26, 2023

400じゃなくて404で良いのでは

@saschanaz
Copy link
Member Author

実は最初に404にしましたが「これクライアントが間違っているから400じゃ?」とか考えて400にしました。でもツイッターとかwikipediaとかみんな404だしでいるので404にします

@saschanaz saschanaz changed the title fix(backend): return HTTP 400 for any invalid api endpoint paths fix(backend): return HTTP 404 for any unknown api endpoint paths Feb 26, 2023
@tamaina tamaina requested a review from syuilo February 27, 2023 05:54
@syuilo
Copy link
Member

syuilo commented Feb 27, 2023

未知のエンドポイントで404返すことはcypress使わずともテストできるけど一旦は良さそう

@syuilo syuilo merged commit 647a018 into misskey-dev:develop Feb 27, 2023
@syuilo
Copy link
Member

syuilo commented Feb 27, 2023

🙏

@saschanaz saschanaz deleted the api-404 branch February 27, 2023 09:01
@saschanaz
Copy link
Member Author

未知のエンドポイントで404返すことはcypress使わずともテストできるけど一旦は良さそう

Jestでもテストできるって意味ですか?👀(旧APIテストがtest/_e2eにあったのでe2eテスト用のcypressにしました)

@syuilo
Copy link
Member

syuilo commented Feb 27, 2023

未知のエンドポイントで404返すことはcypress使わずともテストできるけど一旦は良さそう

Jestでもテストできるって意味ですか?👀(旧APIテストがtest/_e2eにあったのでe2eテスト用のcypressにしました)

以前はJestでこのようなテストを行なっていました

@tamaina
Copy link
Contributor

tamaina commented Feb 27, 2023

APIやサーバー自体のテストはCypressでなくともJestでできそう

@saschanaz
Copy link
Member Author

もうcypress使っているしcypressでもいいのでは(適当)

@syuilo
Copy link
Member

syuilo commented Feb 27, 2023

Misskeyではcypressはブラウザが必要なテスト≒クライアントのテストを行うために使っている

@saschanaz
Copy link
Member Author

#10136 を作成しました

@tamaina
Copy link
Contributor

tamaina commented Feb 27, 2023

cypressはダウンロードがでかいのが難点

@saschanaz
Copy link
Member Author

ブラウザーが必修ならCypress、でないとJest、という感じでいいですか?

@syuilo
Copy link
Member

syuilo commented Feb 27, 2023

ブラウザーが必修ならCypress、でないとJest、という感じでいいですか?

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

存在しないAPIにアクセスするとなぜかHTTP 200が戻ってくる
4 participants