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

Use http.ServeMux instead of chi.Mux #86

Merged
merged 5 commits into from
Apr 2, 2024
Merged

Use http.ServeMux instead of chi.Mux #86

merged 5 commits into from
Apr 2, 2024

Conversation

makiuchi-d
Copy link
Member

Go 1.22から、標準net/httpでもHTTPメソッドの指定とパスパラメータの抽出ができるようになりました。
今までchi(それ以前はgorilla/mux)を使ってやっていましたが、wsnet2ではそんなに高度なことはやっていないので、標準net/httpに書き換えて依存を減らしてみました。

ただ、パスパラメータ部分の正規表現マッチングはnet/httpではできないので、ハンドラ側でチェックする形に変更しました。
これによりマッチしないときのレスポンスが404から400に変わるケースがありますが、正しいクライアントならそもそも起こらないので問題ないはずです。

@makiuchi-d makiuchi-d requested a review from methane April 1, 2024 06:43
number = int32(n)
func (vars JoinVars) roomNumber() (int32, bool) {
v := vars.r.PathValue("roomNumber")
if !numRegexp.Match([]byte(v)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if !numRegexp.Match([]byte(v)) {
if !numRegexp.MatchString(v) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ParseIntは結構入力に対して厳しいので、マイナスになってないか以外のチェックはParseIntのエラーに頼っても良いかもしれません。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

たしかに


for id, valid := range tests {
if IsValidRoomId(id) != valid {
t.Fatalf("IsValidRoomId(%v) wants %v", id, valid)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

これは継続可能なエラーなので、FatalfじゃなくてErrorfでいいかもしれません。
テストがfailするときも、どのテストケースの結果が何になるのかのリストが出た方がバグ修正しやすいので。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

他のところも直せるところありそうですね。
いったんここだけ直しておいて、他のところは別PRで。

)

func init() {
initQueries()
seed, _ := crand.Int(crand.Reader, big.NewInt(math.MaxInt64))
randsrc = rand.New(rand.NewSource(seed.Int64()))

rerid = regexp.MustCompile(idPattern)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
rerid = regexp.MustCompile(idPattern)
rerid = regexp.MustCompile("^[0-9a-f]+$")

@makiuchi-d
Copy link
Member Author

@methane
レビューありがとうございます。対応してみました。

@makiuchi-d makiuchi-d merged commit 1cbc278 into main Apr 2, 2024
5 checks passed
@makiuchi-d makiuchi-d deleted the std-http branch April 2, 2024 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants