6주차 미션 / 서버 4조 이정빈 #50
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pull Request 분석Walkthrough기존의 커스텀 Java 웹 프레임워크에서 Spring Boot 기반 아키텍처로의 마이그레이션입니다. 수동 JDBC 연결 관리 및 저장소 패턴을 제거하고, JPA 기반 데이터 접근 계층, Spring MVC 컨트롤러, 그리고 JSP 뷰를 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant HomeController
participant QuestionDao
participant Database
participant View as JSP View
User->>Browser: 홈페이지 접속
Browser->>HomeController: GET /
HomeController->>QuestionDao: findAll()
QuestionDao->>Database: SELECT * FROM QUESTIONS
Database-->>QuestionDao: Question 목록
QuestionDao-->>HomeController: List<Question>
HomeController->>View: model.addAttribute("questions", ...)
View-->>Browser: home.jsp 렌더링
Browser-->>User: Q&A 목록 표시
sequenceDiagram
actor User
participant Browser
participant UserController
participant UserDao
participant Database
participant Session
User->>Browser: 로그인 시도
Browser->>UserController: POST /user/login (userId, password)
UserController->>UserDao: findByUserId(userId)
UserDao->>Database: SELECT * FROM USERS WHERE user_id = ?
Database-->>UserDao: User 객체
UserDao-->>UserController: User
alt 인증 성공
UserController->>Session: setAttribute("user", user)
UserController-->>Browser: 리다이렉트 /
else 인증 실패
UserController-->>Browser: 리다이렉트 /user/loginFailed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 특별 주의 사항:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (1)
src/main/webapp/user/login.jsp (1)
12-25: CSRF 토큰 누락 확인이 필요합니다.이 로그인 폼도 CSRF 보호가 필요합니다. loginFailed.jsp와 동일하게 Spring Security의 CSRF 설정을 확인해주세요.
🧹 Nitpick comments (25)
java-webMVC.ipr (1)
1-796: IntelliJ IDEA 프로젝트 파일을 버전 관리에서 제외하는 것을 고려하세요.
.ipr파일은 IntelliJ IDEA 전용 프로젝트 설정 파일로, 일반적으로 버전 관리에 포함하지 않는 것이 권장됩니다:
- 다른 IDE를 사용하는 개발자에게는 불필요합니다
- 사용자별 경로(
$USER_HOME$)가 포함되어 있어 환경마다 다를 수 있습니다- 병합 충돌의 원인이 될 수 있습니다
.gitignore에*.ipr,*.iml,.idea/패턴을 추가하여 향후 IDE 설정 파일이 커밋되지 않도록 하는 것을 권장합니다.java-webMVC.iws (1)
1-182: IntelliJ IDEA 워크스페이스 파일을 버전 관리에서 제거하세요.
.iws파일은 IntelliJ IDEA의 워크스페이스 상태를 저장하는 파일로, 개인별 IDE 상태, 최근 파일 목록, 작업 내역 등을 포함합니다. 이러한 파일은 절대 버전 관리에 포함되어서는 안 됩니다:
- 개발자마다 다른 워크스페이스 상태를 가집니다
- 개발 중 빈번하게 변경되어 불필요한 커밋을 유발합니다
- 팀원 간 공유할 필요가 없는 개인 설정입니다
.gitignore에*.iws패턴을 추가하고 이 파일을 Git에서 제거하세요.build.gradle (1)
12-14: 주석 처리된 코드를 제거하세요.11번 줄에서 이미
sourceCompatibility를 설정했으므로, 12-14번 줄의 주석 처리된 코드는 불필요합니다.-//java { -// sourceCompatibility = '17' -//}src/main/webapp/user/loginFailed.jsp (1)
31-31: 상대 경로 대신 절대 경로 사용을 고려하세요.
../js/scripts.js와 같은 상대 경로는 요청 경로에 따라 제대로 로드되지 않을 수 있습니다.다음과 같이 절대 경로로 변경하는 것을 권장합니다:
-<script src="../js/scripts.js"></script> +<script src="/js/scripts.js"></script>src/main/webapp/user/list.jsp (2)
30-30: URL 파라미터 인코딩을 고려하세요.
userId값이 특수 문자를 포함할 경우 URL이 손상될 수 있습니다.
<c:url>태그를 사용하여 안전하게 URL을 생성하는 것을 권장합니다:- <th class="col-md-3"><a href="/user/updateForm?userId=${user.userId}" class="btn btn-success" role="button">수정</a></th> + <th class="col-md-3"> + <c:url value="/user/updateForm" var="updateUrl"> + <c:param name="userId" value="${user.userId}"/> + </c:url> + <a href="${updateUrl}" class="btn btn-success" role="button">수정</a> + </th>
38-38: 상대 경로 대신 절대 경로 사용을 고려하세요.-<script src="../js/scripts.js"></script> +<script src="/js/scripts.js"></script>src/main/webapp/user/login.jsp (1)
29-29: 상대 경로 대신 절대 경로 사용을 고려하세요.-<script src="../js/scripts.js"></script> +<script src="/js/scripts.js"></script>src/main/webapp/user/updateForm.jsp (1)
41-41: 상대 경로 대신 절대 경로 사용을 고려하세요.-<script src="../js/scripts.js"></script> +<script src="/js/scripts.js"></script>src/main/webapp/user/form.jsp (2)
17-31: 입력 유효성 검사 속성 추가를 고려하세요.폼 필드에 클라이언트 측 유효성 검사 속성이 없어 사용자 경험이 저하될 수 있습니다.
required,minlength,pattern등의 HTML5 속성을 추가하는 것을 권장합니다.예시:
<div class="form-floating"> - <input type="text" class="form-control" id="userId" name="userId" placeholder="Id"> + <input type="text" class="form-control" id="userId" name="userId" placeholder="Id" required minlength="3" maxlength="20"> <label for="userId">User Id</label> </div> <div class="form-floating"> - <input type="password" class="form-control" id="password" name="password" placeholder="Password"> + <input type="password" class="form-control" id="password" name="password" placeholder="Password" required minlength="8"> <label for="password">Password</label> </div> <div class="form-floating"> - <input type="text" class="form-control" id="name" name="name" placeholder="name"> + <input type="text" class="form-control" id="name" name="name" placeholder="name" required> <label for="name">Name</label> </div> <div class="form-floating"> - <input type="email" class="form-control" id="email" name="email" placeholder="name@example.com"> + <input type="email" class="form-control" id="email" name="email" placeholder="name@example.com" required> <label for="email">Email address</label> </div>
40-40: 상대 경로 대신 절대 경로 사용을 고려하세요.-<script src="../js/scripts.js"></script> +<script src="/js/scripts.js"></script>src/main/webapp/qna/updateForm.jsp (1)
38-38: 상대 경로 대신 절대 경로 사용을 고려하세요.-<script src="../js/scripts.js"></script> +<script src="/js/scripts.js"></script>src/main/java/jwp/WebServerLauncher.java (1)
3-9: 사용하지 않는 import 문을 제거하세요.
Tomcat,File,Loggerimport가 사용되지 않고 있습니다.다음 import 문들을 제거하세요:
package jwp; -import org.apache.catalina.startup.Tomcat; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; - -import java.io.File; -import java.util.logging.Logger; @SpringBootApplication public class WebServerLauncher {src/main/webapp/qna/form.jsp (2)
19-25: 입력 유효성 검사 속성 추가를 고려하세요.제목과 내용 필드에
required속성을 추가하여 사용자 경험을 개선할 수 있습니다.<div class="form-group"> <label for="title">제목</label> - <input type="text" class="form-control" id="title" name="title" placeholder="제목"/> + <input type="text" class="form-control" id="title" name="title" placeholder="제목" required maxlength="200"/> </div> <div class="form-group"> <label for="contents">내용</label> - <textarea name="contents" id="contents" rows="5" class="form-control"></textarea> + <textarea name="contents" id="contents" rows="5" class="form-control" required></textarea> </div>
35-35: 상대 경로 대신 절대 경로 사용을 고려하세요.-<script src="../js/scripts.js"></script> +<script src="/js/scripts.js"></script>src/main/webapp/home.jsp (1)
37-57: 페이지네이션 동적 구현 권장현재 페이지네이션이 정적으로 하드코딩되어 있습니다. 실제 데이터 개수와 페이지 번호를 기반으로 동적으로 렌더링하는 것을 권장합니다.
src/main/java/jwp/dao/QuestionDao.java (3)
18-22: 불필요한 조회 연산 제거 권장
em.persist()후em.find()를 호출하는 것은 불필요합니다.persist()후 엔티티는 이미 영속 상태이므로 직접 반환할 수 있습니다.@Transactional public Question insert(Question question) throws SQLException { em.persist(question); - return em.find(Question.class, question.getQuestionId()); + return question; }
9-35: SQLException 선언 제거 권장JPA는
SQLException을 던지지 않습니다. 이는 JDBC 기반 구현의 잔재로 보입니다. JPA는PersistenceException계열의 예외를 사용합니다.-import java.sql.SQLException; @Repository @RequiredArgsConstructor public class QuestionDao { private final EntityManager em; @Transactional - public Question insert(Question question) throws SQLException { + public Question insert(Question question) { em.persist(question); return question; } @Transactional - public void update(Question question) throws SQLException { + public void update(Question question) { em.merge(question); } - public List<Question> findAll() throws SQLException { + public List<Question> findAll() { return em.createQuery("select q from Question q", Question.class).getResultList(); } - public Question findByQuestionId(int questionId) throws SQLException { + public Question findByQuestionId(int questionId) { return em.find(Question.class, questionId); } }
8-8: Spring의 @transactional 사용 권장
javax.transaction.Transactional대신org.springframework.transaction.annotation.Transactional을 사용하는 것을 권장합니다. Spring의 어노테이션이 더 많은 기능(전파 방식, 격리 수준 등)을 제공합니다.-import javax.transaction.Transactional; +import org.springframework.transaction.annotation.Transactional;src/main/java/jwp/util/UserSessionUtils.java (1)
10-16: 방어적 코딩 제안 (선택사항)
session매개변수에 대한 null 체크를 추가하면 더 안전합니다. 하지만 현재 구현도 충분히 동작합니다.public static User getUserFromSession(HttpSession session){ + if (session == null) { + return null; + } Object user = session.getAttribute(USER_SESSION_KEY); if(user == null){ return null; } return (User) user; }src/main/webapp/qna/show.jsp (2)
31-35: 작성자 확인 조건 추가 권장현재는 로그인한 모든 사용자에게 수정 링크가 표시됩니다. 질문 작성자와 로그인 사용자가 일치하는 경우에만 수정 링크를 표시하는 것이 좋습니다.
- <c:if test="${not empty sessionScope.user}"> + <c:if test="${not empty sessionScope.user && sessionScope.user.userId == question.writer}"> <li> <a class="link-modify-article" href="/qna/updateForm?questionId=${question.questionId}">수정</a> </li> </c:if>
37-39: 삭제 기능 개선 권장삭제 링크에 대해 다음 개선사항을 권장합니다:
- 작성자만 삭제 링크를 볼 수 있도록 조건 추가
- 삭제 전 확인 대화상자 추가
+ <c:if test="${not empty sessionScope.user && sessionScope.user.userId == question.writer}"> <li> - <a class="link-modify-article" href="/qna/delete?questionId=${question.questionId}">삭제</a> + <a class="link-modify-article" href="/qna/delete?questionId=${question.questionId}" onclick="return confirm('정말 삭제하시겠습니까?');">삭제</a> </li> + </c:if>src/main/java/jwp/controller/HomeController.java (2)
16-26: 주석 처리된 코드 제거 권장이전 구현이 주석으로 남아있습니다. Git 히스토리로 추적 가능하므로 제거하는 것을 권장합니다.
-//public class HomeController implements Controller { -// -// private final QuestionDao questionDao = new QuestionDao(); -// -// @Override -// public String execute(HttpServletRequest req, HttpServletResponse resp) throws Exception { -// List<Question> questions = questionDao.findAll(); -// req.setAttribute("questions", questions); -// return "/home.jsp"; -// } -//} - @Controller @RequestMapping("/") @RequiredArgsConstructor
34-39: 예외 처리 개선 제안 (선택사항)
throws Exception은 너무 포괄적입니다. SQLException을 제거하거나(QuestionDao 수정 시), Spring의 예외 처리 메커니즘을 활용하는 것을 고려하세요.src/main/java/jwp/dao/UserDao.java (1)
8-8: Spring의 @transactional 사용 권장QuestionDao와 동일하게
org.springframework.transaction.annotation.Transactional사용을 권장합니다.src/main/java/jwp/model/User.java (1)
3-11: 클래스 레벨 @Setter 사용 주의JPA 엔티티에 클래스 레벨
@Setter를 사용하면@Id필드를 포함한 모든 필드에 setter가 생성됩니다. 이는 의도하지 않은 수정을 유발할 수 있습니다. 필요한 필드에만 개별적으로@Setter를 적용하거나, 명시적인 setter 메서드를 작성하는 것을 권장합니다.@Entity @Table(name = "USERS") @NoArgsConstructor -@Setter public class User { @Id private String userId; + @Setter private String password; + @Setter private String name; + @Setter private String email;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
src/main/webapp/favicon.icois excluded by!**/*.icosrc/main/webapp/img/KUIT.pngis excluded by!**/*.pngsrc/main/webapp/img/picture.jpegis excluded by!**/*.jpeg
📒 Files selected for processing (39)
.run/WebServerLauncher.run.xml(1 hunks)build.gradle(1 hunks)java-webMVC.ipr(1 hunks)java-webMVC.iws(1 hunks)src/main/java/WebServerLauncher.java(0 hunks)src/main/java/core/db/MemoryUserRepository.java(0 hunks)src/main/java/core/db/Repository.java(0 hunks)src/main/java/core/jdbc/ConnectionManager.java(0 hunks)src/main/java/core/mvc/RequestMapping.java(1 hunks)src/main/java/jwp/WebServerLauncher.java(1 hunks)src/main/java/jwp/controller/HomeController.java(1 hunks)src/main/java/jwp/controller/QnaController.java(1 hunks)src/main/java/jwp/controller/UserController.java(1 hunks)src/main/java/jwp/dao/QuestionDao.java(1 hunks)src/main/java/jwp/dao/UserDao.java(1 hunks)src/main/java/jwp/model/Question.java(1 hunks)src/main/java/jwp/model/User.java(3 hunks)src/main/java/jwp/support/context/ContextLoaderListener.java(1 hunks)src/main/java/jwp/util/UserSessionUtils.java(1 hunks)src/main/resources/application.properties(1 hunks)src/main/webapp/home.jsp(1 hunks)src/main/webapp/include/header.jspf(1 hunks)src/main/webapp/include/navigation.jspf(1 hunks)src/main/webapp/qna/form.jsp(1 hunks)src/main/webapp/qna/show.jsp(3 hunks)src/main/webapp/qna/updateForm.jsp(1 hunks)src/main/webapp/user/form.jsp(1 hunks)src/main/webapp/user/list.jsp(1 hunks)src/main/webapp/user/login.jsp(1 hunks)src/main/webapp/user/loginFailed.jsp(1 hunks)src/main/webapp/user/updateForm.jsp(1 hunks)src/test/java/jwp/model/UserTest.java(0 hunks)webapp/index.html(0 hunks)webapp/qna/form.html(0 hunks)webapp/user/form.html(0 hunks)webapp/user/list.html(0 hunks)webapp/user/list.jsp(0 hunks)webapp/user/login.html(0 hunks)webapp/user/login_failed.html(0 hunks)
💤 Files with no reviewable changes (12)
- webapp/user/list.html
- webapp/user/login.html
- src/main/java/core/db/Repository.java
- src/main/java/core/jdbc/ConnectionManager.java
- src/main/java/WebServerLauncher.java
- webapp/user/form.html
- src/main/java/core/db/MemoryUserRepository.java
- webapp/qna/form.html
- webapp/index.html
- webapp/user/login_failed.html
- webapp/user/list.jsp
- src/test/java/jwp/model/UserTest.java
🧰 Additional context used
🧬 Code graph analysis (7)
src/main/java/jwp/dao/UserDao.java (1)
src/main/java/jwp/dao/QuestionDao.java (1)
Repository(12-36)
src/main/java/jwp/dao/QuestionDao.java (1)
src/main/java/jwp/dao/UserDao.java (1)
Repository(12-35)
src/main/java/jwp/model/User.java (1)
src/main/java/jwp/model/Question.java (1)
Entity(12-97)
src/main/java/jwp/controller/HomeController.java (1)
src/main/java/jwp/controller/QnaController.java (1)
Controller(19-76)
src/main/java/jwp/controller/QnaController.java (2)
src/main/java/jwp/util/UserSessionUtils.java (1)
UserSessionUtils(7-21)src/main/java/jwp/controller/UserController.java (1)
Controller(13-84)
src/main/java/jwp/model/Question.java (1)
src/main/java/jwp/model/User.java (1)
Entity(8-110)
src/main/java/jwp/controller/UserController.java (2)
src/main/java/jwp/util/UserSessionUtils.java (1)
UserSessionUtils(7-21)src/main/java/jwp/controller/QnaController.java (1)
Controller(19-76)
🔇 Additional comments (8)
build.gradle (2)
3-11: Spring Boot 마이그레이션이 잘 구성되었습니다.Spring Boot 플러그인과 의존성 관리 플러그인이 적절히 설정되었고, Java 17 호환성도 명시되어 있습니다.
20-41: 의존성 구성이 적절합니다.Spring Boot Starter Web, Data JPA, JSP 지원(Tomcat Jasper, JSTL), 데이터베이스 드라이버, Lombok, 테스트 의존성이 잘 구성되어 있습니다. Spring Boot의 의존성 관리 기능을 활용하여 버전을 명시하지 않은 점도 좋습니다.
src/main/resources/application.properties (1)
1-14: Spring Boot 설정이 적절합니다.개발 환경을 위한 H2 데이터베이스 설정, JPA/Hibernate 구성, JSP 뷰 리졸버 설정이 올바르게 구성되어 있습니다.
spring.sql.init.mode=always는 개발 단계에서 스키마를 자동으로 초기화하기에 적합합니다.참고:
PhysicalNamingStrategyStandardImpl을 사용하면 엔티티 필드명이 그대로 컬럼명으로 매핑됩니다(camelCase → camelCase). 데이터베이스 스키마와 일치하는지 확인하세요.src/main/webapp/include/header.jspf (1)
1-7: JSP 헤더 프래그먼트가 적절히 구성되었습니다.HTML head 요소가 올바르게 설정되어 있으며, Bootstrap CDN 링크와 커스텀 스타일시트 참조가 포함되어 있습니다. 반응형 디자인을 위한 viewport 메타 태그도 적절히 설정되었습니다.
src/main/java/jwp/WebServerLauncher.java (1)
11-15: Spring Boot 애플리케이션 진입점이 올바르게 구성되었습니다.
@SpringBootApplication어노테이션과SpringApplication.run()호출이 정확합니다.src/main/webapp/include/navigation.jspf (1)
1-52: 전체적으로 잘 구현되었습니다세션 기반 네비게이션 렌더링이 JSTL을 활용하여 깔끔하게 구현되어 있습니다. FAQs, About 등 일부 링크가 플레이스홀더로 남아있지만, 점진적 개발 과정에서 자연스러운 상태입니다.
src/main/java/jwp/util/UserSessionUtils.java (1)
18-20: 메서드 구조가 명확합니다
getUserFromSession을 재사용하는 깔끔한 구현입니다.src/main/java/jwp/model/User.java (1)
61-67: 메서드 구조가 명확합니다두 개의
isSameUser오버로드가 잘 설계되었고, 위임 패턴을 통해 코드 중복을 방지하고 있습니다.
Comment on lines
+1
to
+35
| //package core.mvc; | ||
| // | ||
| //import jwp.controller.*; | ||
| // | ||
| //import java.util.HashMap; | ||
| //import java.util.Map; | ||
| // | ||
| //public class RequestMapping { | ||
| // | ||
| // private static final Map<String, Controller> controllers = new HashMap<>(); | ||
| // | ||
| // static { | ||
| //// controllers.put("/", new HomeController()); | ||
| //// controllers.put("/user/signup", new CreateUserController()); | ||
| //// controllers.put("/user/list", new ListUserController()); | ||
| //// controllers.put("/user/login", new LoginController()); | ||
| //// controllers.put("/user/logout", new LogoutController()); | ||
| //// controllers.put("/user/update", new UpdateUserController()); | ||
| //// controllers.put("/user/updateForm", new UpdateUserFormController()); | ||
| // | ||
| // controllers.put("/user/form", new ForwardController("/user/form.jsp")); | ||
| // controllers.put("/user/loginForm", new ForwardController("/user/login.jsp")); | ||
| // controllers.put("/user/loginFailed", new ForwardController("/user/loginFailed.jsp")); | ||
| // | ||
| //// controllers.put("/qna/form", new CreateQuestionFormController()); | ||
| //// controllers.put("/qna/updateForm", new UpdateQuestionFormController()); | ||
| // controllers.put("/qna/update", new UpdateQuestionController()); | ||
| // controllers.put("/qna/create", new CreateQuestionController()); | ||
| // controllers.put("/qna/show", new ShowQuestionController()); | ||
| // } | ||
| // | ||
| // public Controller getController(String url) { | ||
| // return controllers.get(url); | ||
| // } | ||
| //} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
주석 처리된 전체 파일을 삭제하세요.
이 파일은 완전히 주석 처리되어 있으며 더 이상 사용되지 않습니다. Spring Boot로 마이그레이션하면서 @Controller 어노테이션 기반의 라우팅을 사용하게 되었으므로, 이 수동 RequestMapping 클래스는 필요하지 않습니다. 주석 처리된 코드를 유지하는 것은 코드베이스를 복잡하게 만들고 향후 개발자에게 혼란을 줄 수 있습니다.
파일 전체를 삭제하거나, 레거시 참고용으로 보관이 필요하다면 별도의 문서나 Git 히스토리를 활용하세요.
🤖 Prompt for AI Agents
In src/main/java/core/mvc/RequestMapping.java lines 1-35, the entire file is
commented-out legacy routing code and should be removed; delete this file from
the repository (or, if you must preserve it for historical reference, move it
into a dedicated archive/docs location or convert it into a small README in docs
explaining its deprecation and refer to Git history) so the codebase no longer
contains obsolete commented code.
Comment on lines
+34
to
+37
| public String updateQuestionForm(@RequestParam String questionId, HttpSession session, Model model) throws Exception { | ||
| if (UserSessionUtils.isLogined(session)) { | ||
| return "redirect:/user/loginForm"; | ||
| } |
There was a problem hiding this comment.
로그인 검사 조건이 반대로 적용됨
현재는 로그인한 사용자가 /qna/updateForm에 접근하면 오히려 로그인 페이지로 돌려보내고, 비로그인 사용자는 계속 진행합니다. 조건을 반대로 뒤집어 정상 사용자만 수정 화면에 접근하도록 해주세요.
- if (UserSessionUtils.isLogined(session)) {
+ if (!UserSessionUtils.isLogined(session)) {
return "redirect:/user/loginForm";
}🤖 Prompt for AI Agents
In src/main/java/jwp/controller/QnaController.java around lines 34 to 37, the
login check is inverted: currently it redirects logged-in users to the login
page and allows anonymous users through. Change the condition to check for NOT
logged in (i.e., if (!UserSessionUtils.isLogined(session))) and then return
"redirect:/user/loginForm"; so only unauthenticated users are redirected and
authenticated users proceed to the update form.
Comment on lines
+49
to
+51
| if (!UserSessionUtils.isLogined(session)) { | ||
| return "redirect:/users/loginForm"; | ||
| } |
There was a problem hiding this comment.
로그인 리디렉션 경로 오탈자로 404 발생
비로그인 상태에서 /qna/update를 호출하면 /users/loginForm으로 리디렉션되어 존재하지 않는 경로로 빠집니다. /user/loginForm으로 수정해야 합니다.
- if (!UserSessionUtils.isLogined(session)) {
- return "redirect:/users/loginForm";
+ if (!UserSessionUtils.isLogined(session)) {
+ return "redirect:/user/loginForm";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!UserSessionUtils.isLogined(session)) { | |
| return "redirect:/users/loginForm"; | |
| } | |
| if (!UserSessionUtils.isLogined(session)) { | |
| return "redirect:/user/loginForm"; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/jwp/controller/QnaController.java around lines 49 to 51, the
redirect path for unauthenticated users is incorrect ("/users/loginForm") and
causes a 404; change the redirect target to "/user/loginForm" so the controller
redirects to the correct login form endpoint.
Comment on lines
+63
to
+67
| @PostMapping("/create") | ||
| public String createQuestion(@ModelAttribute Question question) throws Exception { | ||
| Question savedQuestion = questionDao.insert(question); | ||
| System.out.println("saved question id= " + savedQuestion.getQuestionId()); | ||
| return "redirect:/"; |
There was a problem hiding this comment.
질문 생성 시 인증 누락 및 작성자 위조 가능
/qna/create는 세션 확인 없이 넘어와 누구나 질문을 만들 수 있고, 폼 파라미터로 작성자를 임의 입력해 타인 명의로 게시할 수 있습니다. 서버에서 로그인 여부를 검증하고 세션 사용자로 Question을 구성하도록 바꿔주세요.
- public String createQuestion(@ModelAttribute Question question) throws Exception {
- Question savedQuestion = questionDao.insert(question);
- System.out.println("saved question id= " + savedQuestion.getQuestionId());
+ public String createQuestion(@RequestParam String title,
+ @RequestParam String contents,
+ HttpSession session) throws Exception {
+ User loginUser = UserSessionUtils.getUserFromSession(session);
+ if (loginUser == null) {
+ return "redirect:/user/loginForm";
+ }
+ Question question = new Question(loginUser.getUserId(), title, contents, 0);
+ Question savedQuestion = questionDao.insert(question);
return "redirect:/";
}
Comment on lines
+53
to
+57
| @PostMapping("/update") | ||
| public String updateUser(@ModelAttribute User user) throws Exception { | ||
| userDao.update(user); | ||
| return "redirect:/user/list"; | ||
| } |
There was a problem hiding this comment.
세션 검증 없이 사용자 업데이트 허용됨
/user/update가 세션 사용자 검증 없이 임의의 userId로 POST를 받아들이고 있어 누구나 다른 계정을 수정할 수 있습니다. 세션에서 로그인 유저를 꺼내 동일인 여부를 확인한 뒤에만 업데이트하세요.
- public String updateUser(@ModelAttribute User user) throws Exception {
- userDao.update(user);
+ public String updateUser(@ModelAttribute User user, HttpSession session) throws Exception {
+ User loginUser = UserSessionUtils.getUserFromSession(session);
+ if (loginUser == null || !loginUser.getUserId().equals(user.getUserId())) {
+ return "redirect:/user/loginForm";
+ }
+ userDao.update(user);
return "redirect:/user/list";Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/jwp/controller/UserController.java around lines 53 to 57, the
POST /user/update handler accepts any User payload and calls userDao.update
without validating session ownership; change it to retrieve the logged-in user
from the session (e.g. from HttpSession or the security context), compare the
session user's id with the incoming user.id (or overwrite the incoming id with
the session id) and only proceed with userDao.update when they match; if not
authenticated or ids differ, reject the request by redirecting to the login page
or returning an authorization error/forbidden response.
Comment on lines
+17
to
+29
| <input type="hidden" name="questionId" value="${question.questionId}" /> | ||
| <div class="form-group"> | ||
| <label>작성자</label> | ||
| ${question.writer} | ||
| </div> | ||
| <div class="form-group"> | ||
| <label for="title">제목</label> | ||
| <input type="text" class="form-control" id="title" name="title" placeholder="제목" value="${question.title}"/> | ||
| </div> | ||
| <div class="form-group"> | ||
| <label for="contents">내용</label> | ||
| <textarea name="contents" id="contents" rows="5" class="form-control">${question.contents}</textarea> | ||
| </div> |
There was a problem hiding this comment.
XSS 취약점: 질문 데이터 이스케이프 처리가 필요합니다.
${question.writer}, ${question.title}, ${question.contents} 등이 이스케이프 처리 없이 출력되어 XSS 공격에 취약합니다.
<c:out> 태그를 사용하여 안전하게 출력하세요:
<form name="question" method="post" action="/qna/update">
- <input type="hidden" name="questionId" value="${question.questionId}" />
+ <input type="hidden" name="questionId" value="<c:out value='${question.questionId}'/>" />
<div class="form-group">
<label>작성자</label>
- ${question.writer}
+ <c:out value="${question.writer}"/>
</div>
<div class="form-group">
<label for="title">제목</label>
- <input type="text" class="form-control" id="title" name="title" placeholder="제목" value="${question.title}"/>
+ <input type="text" class="form-control" id="title" name="title" placeholder="제목" value="<c:out value='${question.title}'/>"/>
</div>
<div class="form-group">
<label for="contents">내용</label>
- <textarea name="contents" id="contents" rows="5" class="form-control">${question.contents}</textarea>
+ <textarea name="contents" id="contents" rows="5" class="form-control"><c:out value="${question.contents}"/></textarea>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <input type="hidden" name="questionId" value="${question.questionId}" /> | |
| <div class="form-group"> | |
| <label>작성자</label> | |
| ${question.writer} | |
| </div> | |
| <div class="form-group"> | |
| <label for="title">제목</label> | |
| <input type="text" class="form-control" id="title" name="title" placeholder="제목" value="${question.title}"/> | |
| </div> | |
| <div class="form-group"> | |
| <label for="contents">내용</label> | |
| <textarea name="contents" id="contents" rows="5" class="form-control">${question.contents}</textarea> | |
| </div> | |
| <input type="hidden" name="questionId" value="<c:out value='${question.questionId}'/>"/> | |
| <div class="form-group"> | |
| <label>작성자</label> | |
| <c:out value="${question.writer}"/> | |
| </div> | |
| <div class="form-group"> | |
| <label for="title">제목</label> | |
| <input type="text" class="form-control" id="title" name="title" placeholder="제목" value="<c:out value='${question.title}'/>"/> | |
| </div> | |
| <div class="form-group"> | |
| <label for="contents">내용</label> | |
| <textarea name="contents" id="contents" rows="5" class="form-control"><c:out value="${question.contents}"/></textarea> | |
| </div> |
🤖 Prompt for AI Agents
In src/main/webapp/qna/updateForm.jsp around lines 17 to 29, the template prints
question.writer, question.title and question.contents directly which exposes an
XSS vulnerability; replace those raw EL outputs with escaped output using JSTL
<c:out> (or equivalent server-side escaping) for each place where
${question.writer}, ${question.title}, and ${question.contents} are rendered,
ensuring values are HTML-escaped and that any existing attributes (e.g.,
value="...") remain correctly encoded; if rich text is required for contents,
sanitize on input/server-side instead of disabling escaping.
Comment on lines
+22
to
+32
| <c:forEach items="${users}" var="user"> | ||
| <tr> | ||
| <th class="col-md-3">${user.userId} | ||
| </th> | ||
| <th class="col-md-3">${user.name} | ||
| </th> | ||
| <th class="col-md-3">${user.email} | ||
| </th> | ||
| <th class="col-md-3"><a href="/user/updateForm?userId=${user.userId}" class="btn btn-success" role="button">수정</a></th> | ||
| </tr> | ||
| </c:forEach> |
There was a problem hiding this comment.
XSS 취약점: 사용자 데이터 출력 시 이스케이프 처리가 필요합니다.
${user.userId}, ${user.name}, ${user.email} 등의 출력이 이스케이프 처리 없이 직접 렌더링되고 있습니다. 악의적인 사용자가 스크립트를 포함한 데이터를 입력하면 XSS 공격에 노출될 수 있습니다.
JSTL의 <c:out> 태그를 사용하여 이스케이프 처리를 적용하세요:
<c:forEach items="${users}" var="user">
<tr>
- <th class="col-md-3">${user.userId}
+ <th class="col-md-3"><c:out value="${user.userId}"/>
</th>
- <th class="col-md-3">${user.name}
+ <th class="col-md-3"><c:out value="${user.name}"/>
</th>
- <th class="col-md-3">${user.email}
+ <th class="col-md-3"><c:out value="${user.email}"/>
</th>
<th class="col-md-3"><a href="/user/updateForm?userId=${user.userId}" class="btn btn-success" role="button">수정</a></th>
</tr>
</c:forEach>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <c:forEach items="${users}" var="user"> | |
| <tr> | |
| <th class="col-md-3">${user.userId} | |
| </th> | |
| <th class="col-md-3">${user.name} | |
| </th> | |
| <th class="col-md-3">${user.email} | |
| </th> | |
| <th class="col-md-3"><a href="/user/updateForm?userId=${user.userId}" class="btn btn-success" role="button">수정</a></th> | |
| </tr> | |
| </c:forEach> | |
| <c:forEach items="${users}" var="user"> | |
| <tr> | |
| <th class="col-md-3"><c:out value="${user.userId}"/> | |
| </th> | |
| <th class="col-md-3"><c:out value="${user.name}"/> | |
| </th> | |
| <th class="col-md-3"><c:out value="${user.email}"/> | |
| </th> | |
| <th class="col-md-3"><a href="/user/updateForm?userId=${user.userId}" class="btn btn-success" role="button">수정</a></th> | |
| </tr> | |
| </c:forEach> |
🤖 Prompt for AI Agents
In src/main/webapp/user/list.jsp around lines 22–32, the JSP directly renders
${user.userId}, ${user.name}, and ${user.email} which allows XSS; replace those
inline EL outputs with JSTL <c:out> (e.g., use c:out value="${user.name}" etc.)
to perform HTML-escaping, ensure the JSTL core taglib is declared at the top of
the JSP if not already, and also escape/encode the userId used in the updateForm
link (use c:url or c:out for the query value) so no unescaped user data is
injected into attributes or markup.
Comment on lines
+14
to
+27
| <form name="login" method="post" action="/user/login"> | ||
| <h1 class="h3 mb-3 fw-normal">Please sign in</h1> | ||
|
|
||
| <div class="form-floating"> | ||
| <input type="text" class="form-control" id="loginId" name="userId" placeholder="id"> | ||
| <label for="loginId">ID</label> | ||
| </div> | ||
| <div class="form-floating"> | ||
| <input type="password" class="form-control" id="loginPassword" name="password" placeholder="Password"> | ||
| <label for="loginPassword">Password</label> | ||
| </div> | ||
|
|
||
| <button class="w-100 btn btn-lg btn-primary" type="submit">Sign in</button> | ||
| </form> |
There was a problem hiding this comment.
🧩 Analysis chain
CSRF 토큰 누락 확인이 필요합니다.
Spring Security가 활성화되어 있다면 CSRF 토큰이 자동으로 처리되지만, 그렇지 않은 경우 CSRF 공격에 취약할 수 있습니다. 프로젝트에 Spring Security CSRF 보호가 설정되어 있는지 확인해주세요.
다음 스크립트로 Spring Security 설정을 확인해보세요:
🏁 Script executed:
#!/bin/bash
# Spring Security 설정 파일 확인
fd -e java -e xml | xargs rg -l "SecurityConfig|WebSecurityConfigurerAdapter|SecurityFilterChain"
# CSRF 관련 설정 확인
rg -n "csrf\(\)" --type javaLength of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# 프로젝트 구조 및 의존성 확인
echo "=== Build configuration files ==="
fd -e pom.xml -e build.gradle -e gradle.properties
echo "=== Project structure ==="
find . -maxdepth 2 -type f -name "*.xml" -o -name "*.properties" | head -20
echo "=== Spring dependency check ==="
fd pom.xml -x cat {} \; | head -50
Length of output: 124
🏁 Script executed:
#!/bin/bash
# JSP 파일에서 CSRF 토큰 처리 확인
echo "=== Search for CSRF token in JSP files ==="
rg -n "csrf|CSRF|_csrf" --type html
echo "=== Search for login-related controllers ==="
rg -n "login" --type java -C3
echo "=== Check web.xml for security configuration ==="
fd web.xml -x cat {}Length of output: 9361
🏁 Script executed:
#!/bin/bash
# Spring 설정 파일 확인
echo "=== Check application configuration files ==="
fd -e properties -e yml -e yaml
echo "=== Check for Filter classes ==="
rg -n "Filter|SecurityFilter" --type java -C2
echo "=== Check ServletConfig or WebConfig ==="
rg -n "ServletConfig|WebConfig|WebMvcConfigurer" --type java -C2
echo "=== List all Java config files ==="
fd -e java -path "*/config/*"Length of output: 4774
🏁 Script executed:
#!/bin/bash
# 애플리케이션 설정 파일 내용 확인
echo "=== application.properties content ==="
cat src/main/resources/application.properties
echo ""
echo "=== Check for any CSRF-related code in controllers ==="
rg -n "csrf|CSRF|token" --type javaLength of output: 566
로그인 폼에 CSRF 토큰 추가 필요
현재 프로젝트에 Spring Security가 설정되어 있지 않으며, 로그인 폼(src/main/webapp/user/loginFailed.jsp 14-27줄, src/main/webapp/user/login.jsp)에 CSRF 토큰이 없어 CSRF 공격에 취약합니다.
POST 요청을 처리하는 /user/login 엔드포인트(UserController.java 26-36줄)에서 토큰 검증이 이루어지지 않으므로, 다음 중 하나를 구현하세요:
- Spring Security 활성화 및 CSRF 필터 적용
- 또는 수동으로 CSRF 토큰 생성/검증 메커니즘 추가
🤖 Prompt for AI Agents
In src/main/webapp/user/loginFailed.jsp lines 14-27 (and the companion
src/main/webapp/user/login.jsp), the login form posts to /user/login without a
CSRF token making the endpoint vulnerable; fix by either enabling Spring
Security with CSRF protection or adding a manual token flow: generate a
cryptographically secure CSRF token on the server, store it in the user session
(or secure cookie), render it into the login forms as a hidden input, and then
validate that token in UserController when handling POST /user/login (reject
requests with missing/invalid tokens and rotate tokens on successful auth);
ensure tokens are unique per session, have sufficient entropy, and that
sensitive error paths do not leak token values.
Comment on lines
+15
to
+31
| <input type="text" class="form-control" value="${user.userId}" id="userId" name="userId" | ||
| placeholder="Id" readonly> | ||
| <label for="userId">User Id</label> | ||
| </div> | ||
| <div class="form-floating"> | ||
| <input type="password" class="form-control" value="${user.password}" id="password" name="password" | ||
| placeholder="Password"> | ||
| <label for="password">Password</label> | ||
| </div> | ||
| <div class="form-floating"> | ||
| <input type="text" class="form-control" value="${user.name}" id="name" name="name" placeholder="name"> | ||
| <label for="name">Name</label> | ||
| </div> | ||
| <div class="form-floating"> | ||
| <input type="email" class="form-control" value="${user.email}" id="email" name="email" | ||
| placeholder="name@example.com"> | ||
| <label for="email">Email address</label> |
There was a problem hiding this comment.
XSS 취약점: 사용자 입력 데이터 이스케이프 처리가 필요합니다.
사용자 데이터가 이스케이프 처리 없이 출력되고 있어 XSS 공격에 취약합니다.
<c:out> 태그를 사용하여 안전하게 출력하세요:
<div class="form-floating">
- <input type="text" class="form-control" value="${user.userId}" id="userId" name="userId"
+ <input type="text" class="form-control" value="<c:out value='${user.userId}'/>" id="userId" name="userId"
placeholder="Id" readonly>
<label for="userId">User Id</label>
</div>
<div class="form-floating">
<input type="password" class="form-control" id="password" name="password"
placeholder="Password">
<label for="password">Password</label>
</div>
<div class="form-floating">
- <input type="text" class="form-control" value="${user.name}" id="name" name="name" placeholder="name">
+ <input type="text" class="form-control" value="<c:out value='${user.name}'/>" id="name" name="name" placeholder="name">
<label for="name">Name</label>
</div>
<div class="form-floating">
- <input type="email" class="form-control" value="${user.email}" id="email" name="email"
+ <input type="email" class="form-control" value="<c:out value='${user.email}'/>" id="email" name="email"
placeholder="name@example.com">
<label for="email">Email address</label>
</div>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/webapp/user/updateForm.jsp around lines 15 to 31, the JSP outputs
user-provided values with raw EL (e.g. ${user.email}) which is vulnerable to
XSS; replace those raw EL usages with JSTL <c:out> to escape the values (e.g.
use <c:out value="${user.email}" /> for value attributes), ensure the JSTL core
taglib is declared at the top of the JSP
(uri="http://java.sun.com/jsp/jstl/core" prefix="c"), and avoid rendering
unescaped user input anywhere in the form (apply to userId, password, name,
email inputs).
Comment on lines
+20
to
+22
| <input type="password" class="form-control" value="${user.password}" id="password" name="password" | ||
| placeholder="Password"> | ||
| <label for="password">Password</label> |
There was a problem hiding this comment.
보안 문제: 비밀번호가 평문으로 노출됩니다.
비밀번호 필드에 value="${user.password}"를 사용하면 HTML 소스 코드에 비밀번호가 평문으로 노출되어 심각한 보안 위험이 발생합니다. 비밀번호 변경 폼에서는 기존 비밀번호를 표시하지 말고, 사용자가 새 비밀번호를 입력하도록 해야 합니다.
다음과 같이 수정하세요:
<div class="form-floating">
- <input type="password" class="form-control" value="${user.password}" id="password" name="password"
- placeholder="Password">
+ <input type="password" class="form-control" id="password" name="password"
+ placeholder="새 비밀번호 (변경하지 않으려면 비워두세요)">
<label for="password">Password</label>
</div>서버 측 로직도 비밀번호 필드가 비어있으면 변경하지 않도록 수정이 필요합니다.
🤖 Prompt for AI Agents
In src/main/webapp/user/updateForm.jsp around lines 20-22, the password input
currently injects the user's password into the value attribute which exposes
plaintext credentials; remove the value="${user.password}" so the password field
is empty (allow user to enter a new password) and add attributes like
autocomplete="new-password" if desired; additionally update the server-side
update handler to treat an empty password field as "no change" (do not overwrite
existing password when the submitted password is blank) and only hash+persist a
new password when a non-empty value is provided.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
릴리스 노트