[fix] invalid URL - Githubissues

42trans / ft_transcendence

0 stars 0 forks source link

[fix] invalid URL #276

Closed ak0327 closed 2 months ago

ak0327 commented 3 months ago

概要

URLの検証を追加しました。不正なURL, parameterであれば/app/に画面、URL共に遷移するようになっていると思います
ルーター周りの負債が蓄積してきた感が否めません...他の最低限の要件を確実にクリアしてから、リファクタリングしたいと思っています
PRの新規実装・変更が2-300行、テストが300行程度だと思います。テスト追加＆修正で変更が大きめに出ています。
主な検証パラメータはAPIテストに記載しています 1563c3a bc968a5 41fd87c

URLパラメータの整合性評価を追加

/url/:intParam/に文字列で表示バグ（/app/game/match/hoge/, /app/dm/hoge/）
- Djangoのルーティングでintが期待されているため、文字列の入力で404に飛ばされているらしい
int -> strとし、int(intParam)の変換で整数でなければtopに飛ばすなどの処理を追加。とりあえず何かしら表示されるようになった。 b502189
djangoのviewで遷移するとルーター経由しないため、表示内容やURLに不整合が生じる。getSelectedRoute()でパラメータの整合性を評価し、不正なURLの場合 /app/invalid/ -> /app/ に変更 4c150e6
- UserInfo: userIdの判定 a4de784
- DM: dmUserIdの判定 0376e60
- Tournament: matchIdの判定 2444a6e

不正なURLの取り扱いを修正

/app/invalid-url/ -> /app/
- routeTableにマッチしない不正なURLはgetNextPath()でtopを返すよう変更。不正なURLの場合はgetSelectedRoute()がTOPを返すことを利用しているが、この関数を複数回呼ぶのは違和感。余裕があればリファクタリングしたいが... 9f0d645

URLパラメータをnickname -> idベースに変更（リンク変更漏れの修正含む）

/user-info/<nickname>/ -> /user-info/<user_id>/ fbcd948
UserInfo: DM withのURLをidに変更 a7b0631

memo

余力があれば、login userの場合に、DMのinvalidは/dm/, matchIdのinvalidは/tournament/に遷移させたいです。ゲストの場合はapp or login遷移の分岐があり、少々面倒なので未着手です。
django view <-> JSの往復が冗長感が否めません...（初めからJSメインで実装しておくべきだったのか？）
APIでURLパラメータ(userIdなど)の判定を行っているため、consoleに400や401が出てきます

ak0327 commented 3 months ago

とりあえずは最低ライン URLは不正なパラメータのままだが、画面表示はtopに。

before
after

ak0327 commented 2 months ago

log off忘れてたので追加でpushしました

uminomae commented 2 months ago

djangoのviewで遷移するということを「原則禁止」しないといけないですよね。SPAの定義からいくと。遷移させたいmsgや301要求とかでブラウザに遷移してほしいことを要求するようなDjangoの実装を方針だてて書くべきだったのかなと、今さら。

uminomae commented 2 months ago

APIでURLパラメータ(userIdなど)の判定を行っているため、consoleに400や401が出てきます

/tournament/ への遷移、そこからROUNDの遷移で400が出ます。これは２回目の提出では無くした方が良いですね。

Django = 遷移したい時はブラウザに依頼するstatusかmsg SPA = case（戻る、直打ちURLなど）ごとの　brefore遷移, after遷移系メソッドを備えた状態パターンの設計とかでしょか。ここまでのバグの出方で優先したいことだと。

スクリーンショット 2024-06-19 4 53 45