Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,18 @@ reviews:
path_instructions:
- path: "src/main/java/**/*.java"
instructions: |
日本語でレビューしてください。これは初学者も読むデモアプリのため、以下を重視してください。
- セキュリティ: ユーザー入力のURLをサーバー側で取得する処理(jsoup等)におけるSSRF対策を確認する。
内部IP・プライベートアドレス・loopback・クラウドのメタデータエンドポイントへのアクセスがブロック
されているか、スキーム(https/http)が制限されているか、リダイレクト追従の安全性を検証する。
SQLインジェクションやその他のインジェクションの可能性も指摘する。
- 入力検証: @RequestParam 等の外部入力に対するバリデーション(必須・形式・長さ)が網羅されているか確認する。
- 入力検証: @RequestParam 等の外部入力に対するバリデーション(必須・形式・長さ)が網羅されているか確認する。HTML側の制限だけでなく、サーバー側も確実に確認すること
- エラーハンドリング: 例外処理・エラー応答・リダイレクトの方針が一貫しているか確認する。
例外の握りつぶしや、ユーザーに不適切な情報を返していないか指摘する。
- ログ: 個人情報(PII)や秘密情報(認証情報・トークン等)がログに出力されていないか確認する。
- 可読性: 初学者を対象としているため、意図が伝わる十分なコメントがあるか確認し、不足していれば補足を提案する。
- path: "src/main/resources/templates/**/*.html"
instructions: |
日本語でレビューしてください。Thymeleafテンプレートでは以下を重視してください。
Thymeleafテンプレートでは以下を重視してください。
- XSS/出力エスケープ: th:utext などの非エスケープ出力の誤用がないか、ユーザー入力由来の値
(タイトル・URL・OGP画像URL等)が適切にエスケープされているか確認する。
- アクセシビリティ: WCAG 2.1 AA に沿って、img の alt 属性、フォーム要素のラベル、aria 属性、
Expand All @@ -33,7 +32,6 @@ reviews:
初学者向けに分かりにくい箇所はコメントや簡潔化を提案する。
- path: "**/*.sql"
instructions: |
日本語でレビューしてください。SQL/スキーマ定義では以下を重視してください。
- スキーマ設計/制約: NOT NULL、デフォルト値、主キー、一意制約などが適切に設定されているか確認する。
- インデックス: 頻繁に検索・結合されるカラムや外部キーに対するインデックス不足を確認する。
- パフォーマンス: フルスキャンを誘発する設計や非効率なクエリがないか確認する。
Expand All @@ -42,7 +40,6 @@ reviews:
必ず検討し、新規テーブルが本当に必要かを指摘する。
- path: "**/*.md"
instructions: |
日本語でレビューしてください。ドキュメントでは以下を重視してください。
- リンク切れ: リンク先が有効か確認する。
- 見出し階層: 見出しレベルの階層が適切で飛び級がないか確認する。
- コードサンプルの正確さ: 記載されたコマンド・コード例が実装と一致し、実際に動作するか確認する。
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
build/
bookmarks.db
*.db-journal
.DS_Store
111 changes: 111 additions & 0 deletions src/main/java/com/example/bookmark/BookmarkController.java
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
package com.example.bookmark;

import java.io.IOException;
import java.util.Arrays;
import java.util.Objects;
import java.util.stream.Collectors;

import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.servlet.mvc.support.RedirectAttributes;

@Controller
public class BookmarkController {
private static final int TITLE_MAX_LENGTH = 100;
private static final int DESCRIPTION_MAX_LENGTH = 300;

private final BookmarkRepository bookmarkRepository;
private final BookmarkMetadataFetcher metadataFetcher;

Expand Down Expand Up @@ -51,4 +58,108 @@ public String add(@RequestParam String url, RedirectAttributes redirectAttribute
}
return "redirect:/";
}

@GetMapping("/bookmarks/{id}/edit")
public String edit(@PathVariable long id, Model model, RedirectAttributes redirectAttributes) {
return bookmarkRepository.findById(id)
.map(bookmark -> {
if (!model.containsAttribute("title")) {
model.addAttribute("title", bookmark.title());
}
if (!model.containsAttribute("description")) {
model.addAttribute("description", bookmark.description());
}
if (!model.containsAttribute("tags")) {
model.addAttribute("tags", bookmark.tags());
}
model.addAttribute("bookmark", bookmark);
return "edit";
})
.orElseGet(() -> {
redirectAttributes.addFlashAttribute("error", "ブックマークが見つかりません。");
return "redirect:/";
});
}

@PostMapping("/bookmarks/{id}")
public String update(
@PathVariable long id,
@RequestParam String title,
@RequestParam(required = false) String description,
@RequestParam(required = false) String tags,
RedirectAttributes redirectAttributes
) {
if (bookmarkRepository.findById(id).isEmpty()) {
redirectAttributes.addFlashAttribute("error", "ブックマークが見つかりません。");
return "redirect:/";
}
Comment on lines +92 to +95
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

更新結果を最終判定に使ってください。

存在確認後に別リクエストで削除されると、更新失敗でも成功扱いになります。update(...) の戻り値で判定してエラーを返すようにしてください。

💡 修正案
-        if (bookmarkRepository.findById(id).isEmpty()) {
-            redirectAttributes.addFlashAttribute("error", "ブックマークが見つかりません。");
-            return "redirect:/";
-        }
-
         BookmarkForm form = normalize(title, description, tags);
         String validationError = validate(form);
         if (validationError != null) {
             redirectAttributes.addFlashAttribute("error", validationError);
             redirectAttributes.addFlashAttribute("title", title);
             redirectAttributes.addFlashAttribute("description", description);
             redirectAttributes.addFlashAttribute("tags", tags);
             return "redirect:/bookmarks/" + id + "/edit";
         }
 
-        bookmarkRepository.update(id, form.title(), form.description(), form.tags());
+        if (!bookmarkRepository.update(id, form.title(), form.description(), form.tags())) {
+            redirectAttributes.addFlashAttribute("error", "ブックマークが見つかりません。");
+        }
         return "redirect:/";

Also applies to: 107-108

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/example/bookmark/BookmarkController.java` around lines 92 -
95, Current code checks existence with bookmarkRepository.findById(id) before
calling update/delete, which can race with concurrent requests; instead call the
update(...) method and use its return value (or affected-row count) to determine
success and set redirectAttributes.addFlashAttribute("error", ...) when
update(...) indicates failure; remove reliance on the prior existence check (or
keep it only for early fast-fail) but always treat update(...)'s
boolean/rows-updated as the final authority for both the update(...) call(s) and
the similar case around the lines 107-108, returning "redirect:/" with an error
flash when the update indicates no rows were changed.


BookmarkForm form = normalize(title, description, tags);
String validationError = validate(form);
if (validationError != null) {
redirectAttributes.addFlashAttribute("error", validationError);
redirectAttributes.addFlashAttribute("title", title);
redirectAttributes.addFlashAttribute("description", description);
redirectAttributes.addFlashAttribute("tags", tags);
return "redirect:/bookmarks/" + id + "/edit";
}

bookmarkRepository.update(id, form.title(), form.description(), form.tags());
return "redirect:/";
}

@PostMapping("/bookmarks/{id}/delete")
public String delete(@PathVariable long id, RedirectAttributes redirectAttributes) {
if (!bookmarkRepository.delete(id)) {
redirectAttributes.addFlashAttribute("error", "ブックマークが見つかりません。");
}
return "redirect:/";
}

private static BookmarkForm normalize(String title, String description, String tags) {
return new BookmarkForm(
trimToNull(title),
trimToNull(description),
normalizeTags(tags)
);
}

private static String validate(BookmarkForm form) {
if (form.title() == null) {
return "タイトルを入力してください。";
}
if (codePointLength(form.title()) > TITLE_MAX_LENGTH) {
return "タイトルは100文字以下で入力してください。";
}
if (form.description() != null && codePointLength(form.description()) > DESCRIPTION_MAX_LENGTH) {
return "メモは300文字以下で入力してください。";
}
return null;
}

private static String normalizeTags(String tags) {
if (tags == null) {
return null;
}
String normalized = Arrays.stream(tags.split(","))
.map(BookmarkController::trimToNull)
.filter(Objects::nonNull)
.collect(Collectors.joining(","));
return normalized.isEmpty() ? null : normalized;
}

private static String trimToNull(String value) {
if (value == null) {
return null;
}
String trimmed = value.trim();
return trimmed.isEmpty() ? null : trimmed;
}

private static int codePointLength(String value) {
return value.codePointCount(0, value.length());
}

private record BookmarkForm(String title, String description, String tags) {
}
}
52 changes: 42 additions & 10 deletions src/main/java/com/example/bookmark/BookmarkRepository.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
package com.example.bookmark;

import java.util.List;
import java.util.Optional;

import org.springframework.dao.EmptyResultDataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.stereotype.Repository;

@Repository
public class BookmarkRepository {
private final JdbcTemplate jdbcTemplate;
private static final org.springframework.jdbc.core.RowMapper<Bookmark> BOOKMARK_ROW_MAPPER = (rs, rowNum) -> new Bookmark(
rs.getLong("id"),
rs.getString("title"),
rs.getString("url"),
rs.getString("description"),
rs.getString("tags"),
rs.getString("ogp_image_url"),
rs.getString("created_at"),
rs.getString("updated_at")
);

public BookmarkRepository(JdbcTemplate jdbcTemplate) {
this.jdbcTemplate = jdbcTemplate;
Expand All @@ -18,16 +30,19 @@ public List<Bookmark> findAll() {
SELECT id, title, url, description, tags, ogp_image_url, created_at, updated_at
FROM bookmarks
ORDER BY id DESC
""", (rs, rowNum) -> new Bookmark(
rs.getLong("id"),
rs.getString("title"),
rs.getString("url"),
rs.getString("description"),
rs.getString("tags"),
rs.getString("ogp_image_url"),
rs.getString("created_at"),
rs.getString("updated_at")
));
""", BOOKMARK_ROW_MAPPER);
}

public Optional<Bookmark> findById(long id) {
try {
return Optional.ofNullable(jdbcTemplate.queryForObject("""
SELECT id, title, url, description, tags, ogp_image_url, created_at, updated_at
FROM bookmarks
WHERE id = ?
""", BOOKMARK_ROW_MAPPER, id));
} catch (EmptyResultDataAccessException ex) {
return Optional.empty();
}
}

public void add(String title, String url, String description, String tags, String ogpImageUrl) {
Expand All @@ -36,4 +51,21 @@ INSERT INTO bookmarks (title, url, description, tags, ogp_image_url)
VALUES (?, ?, ?, ?, ?)
""", title, url, description, tags, ogpImageUrl);
}

public boolean update(long id, String title, String description, String tags) {
int updatedRows = jdbcTemplate.update("""
UPDATE bookmarks
SET title = ?, description = ?, tags = ?, updated_at = datetime('now')
WHERE id = ?
""", title, description, tags, id);
return updatedRows > 0;
}

public boolean delete(long id) {
int deletedRows = jdbcTemplate.update("""
DELETE FROM bookmarks
WHERE id = ?
""", id);
return deletedRows > 0;
}
}
Loading