From c5b175554915e73bd56112a8b97c2aeb3dd3d3c7 Mon Sep 17 00:00:00 2001 From: gousaiyang Date: Sun, 16 Jul 2017 18:05:16 +0800 Subject: [PATCH] Eliminate business logic in controller & fix XSS bug --- .../bookstore/action/AdminBookAction.java | 24 +--- .../bookstore/action/AdminCategoryAction.java | 13 +- .../bookstore/action/AdminUserAction.java | 34 +---- .../java/bookstore/action/AuthAction.java | 11 +- .../java/bookstore/action/CartAction.java | 10 +- .../java/bookstore/action/ProfileAction.java | 18 +-- .../bookstore/action/UploadImageAction.java | 36 +++--- .../java/bookstore/service/AppService.java | 28 +++- .../service/impl/AppServiceImpl.java | 122 ++++++++++++++++-- src/main/webapp/js/helper/message.js | 8 +- 10 files changed, 175 insertions(+), 129 deletions(-) diff --git a/src/main/java/bookstore/action/AdminBookAction.java b/src/main/java/bookstore/action/AdminBookAction.java index 842a2d9..6582318 100644 --- a/src/main/java/bookstore/action/AdminBookAction.java +++ b/src/main/java/bookstore/action/AdminBookAction.java @@ -191,18 +191,9 @@ public String addBook() { retJson = vd.getFailureMessage(); return ERROR; } - - Book book = new Book(); - book.setName(getName()); - book.setImage(getImage()); - book.setAuthor(getAuthor()); - book.setPress(getPress()); - book.setPrice((int)(Float.parseFloat(getPrice()) * 100)); - book.setStock(Integer.parseInt(getStock())); - book.setDescription(getDescription()); - appService.addBook(book); - - retJson = new SuccessMessage(book.getId()); + + retJson = new SuccessMessage(appService.addBook(getName(), getImage(), getAuthor(), getPress(), + getPrice(), getStock(), getDescription())); return SUCCESS; } @@ -244,14 +235,7 @@ public String updateBook() { return NONE; } - book.setName(getName()); - book.setImage(getImage()); - book.setAuthor(getAuthor()); - book.setPress(getPress()); - book.setPrice((int)(Float.parseFloat(getPrice()) * 100)); - book.setStock(Integer.parseInt(getStock())); - book.setDescription(getDescription()); - appService.updateBook(book); + appService.updateBook(book, getName(), getImage(), getAuthor(), getPress(), getPrice(), getStock(), getDescription()); retJson = new SuccessMessage(); return SUCCESS; diff --git a/src/main/java/bookstore/action/AdminCategoryAction.java b/src/main/java/bookstore/action/AdminCategoryAction.java index 5b550a0..118d226 100644 --- a/src/main/java/bookstore/action/AdminCategoryAction.java +++ b/src/main/java/bookstore/action/AdminCategoryAction.java @@ -137,11 +137,8 @@ public String addCategory() { retJson = vd.getFailureMessage(); return ERROR; } - - Category category = new Category(getName()); - appService.addCategory(category); - - retJson = new SuccessMessage(category.getId()); + + retJson = new SuccessMessage(appService.addCategory(getName())); return SUCCESS; } @@ -173,8 +170,7 @@ public String updateCategory() { return NONE; } - category.setName(getName()); - appService.updateCategory(category); + appService.updateCategory(category, getName()); retJson = new SuccessMessage(); return SUCCESS; @@ -249,8 +245,7 @@ public String addBookToCategory() { return ERROR; } - bc = new BookCategory(category.getId(), book.getId()); - appService.addBC(bc); + appService.addBC(category.getId(), book.getId()); retJson = new SuccessMessage(); return SUCCESS; diff --git a/src/main/java/bookstore/action/AdminUserAction.java b/src/main/java/bookstore/action/AdminUserAction.java index 2396378..8b074aa 100644 --- a/src/main/java/bookstore/action/AdminUserAction.java +++ b/src/main/java/bookstore/action/AdminUserAction.java @@ -1,6 +1,5 @@ package bookstore.action; -import java.util.ArrayList; import java.util.List; import bookstore.model.User; @@ -8,7 +7,6 @@ import bookstore.model.result.SuccessMessage; import bookstore.model.result.UserDetail; import bookstore.service.AppService; -import bookstore.util.PasswordUtil; import bookstore.util.StringUtil; import bookstore.util.Validator; @@ -228,17 +226,9 @@ public String addUser() throws Exception { retJson = new FailureMessage("用户名 " + getUsername() + " 已经存在。"); return ERROR; } - - User user = new User(); - user.setUsername(getUsername()); - user.setPassword(PasswordUtil.passwordHash(getPassword())); - user.setNickname(getNickname()); - user.setAvatar(getAvatar()); - user.setBalance((int)(Float.parseFloat(getBalance()) * 100)); - user.setRole(getRole().equals("1")); - appService.addUser(user); - retJson = new SuccessMessage(user.getId()); + retJson = new SuccessMessage(appService.addUser(getUsername(), getPassword(), getNickname(), + getAvatar(), getBalance(), getRole())); return SUCCESS; } @@ -301,14 +291,7 @@ public String updateUser() throws Exception { return ERROR; } - user.setUsername(getUsername()); - if (!getPassword().equals("")) - user.setPassword(PasswordUtil.passwordHash(getPassword())); - user.setNickname(getNickname()); - user.setAvatar(getAvatar()); - user.setBalance((int)(Float.parseFloat(getBalance()) * 100)); - user.setRole(getRole().equals("1")); - appService.updateUser(user); + appService.updateUser(user, getUsername(), getPassword(), getNickname(), getAvatar(), getBalance(), getRole()); retJson = new SuccessMessage(); return SUCCESS; @@ -403,15 +386,8 @@ public String updateAddress() throws Exception { retJson = new FailureMessage("收货地址数组格式不正确"); return ERROR; } - - List newAddresses = new ArrayList(); - for (String address: addressArray) { - String addr = address.trim(); - if (!addr.isEmpty()) - newAddresses.add(addr); - } - - appService.updateUserAddress(userId, newAddresses); + + appService.updateUserAddress(userId, addressArray); retJson = new SuccessMessage(); return SUCCESS; diff --git a/src/main/java/bookstore/action/AuthAction.java b/src/main/java/bookstore/action/AuthAction.java index 763f423..c2aec3a 100644 --- a/src/main/java/bookstore/action/AuthAction.java +++ b/src/main/java/bookstore/action/AuthAction.java @@ -91,16 +91,9 @@ public String doRegister() { return ERROR; } - User user = new User(); - user.setUsername(getUsername()); - user.setPassword(PasswordUtil.passwordHash(getPassword())); - user.setNickname(getUsername()); - user.setAvatar(""); - user.setBalance(0); - user.setRole(false); - appService.addUser(user); + Integer newUserId = appService.addUser(getUsername(), getPassword(), getUsername(), "", "0", "0"); - session().setAttribute("user", user); + session().setAttribute("user", appService.getUserById(newUserId)); retJson = new SuccessMessage(); return SUCCESS; diff --git a/src/main/java/bookstore/action/CartAction.java b/src/main/java/bookstore/action/CartAction.java index 07b7b2f..516ed7a 100644 --- a/src/main/java/bookstore/action/CartAction.java +++ b/src/main/java/bookstore/action/CartAction.java @@ -118,10 +118,7 @@ public String addToCart() { return ERROR; } - Order cart = appService.getUserCart(user.getId()); - OrderItem item = new OrderItem(cart.getId(), bookId, Integer.parseInt(getQuantity())); - - appService.addOrderItem(item); + appService.addItemToCart(user, bookId, Integer.parseInt(getQuantity())); retJson = new SuccessMessage(); return SUCCESS; @@ -174,8 +171,7 @@ public String updateCartItem() { return "forbidden"; } - item.setQuantity(Integer.parseInt(getQuantity())); - appService.updateOrderItem(item); + appService.updateOrderItem(item, Integer.parseInt(getQuantity())); retJson = new SuccessMessage(); return SUCCESS; @@ -236,7 +232,7 @@ public String payCart() { return LOGIN; } - retJson = appService.payOrder(appService.getUserCart(user.getId())); + retJson = appService.payCart(user); return SUCCESS; } diff --git a/src/main/java/bookstore/action/ProfileAction.java b/src/main/java/bookstore/action/ProfileAction.java index bb0e717..1371890 100644 --- a/src/main/java/bookstore/action/ProfileAction.java +++ b/src/main/java/bookstore/action/ProfileAction.java @@ -1,13 +1,11 @@ package bookstore.action; -import java.util.ArrayList; import java.util.List; import bookstore.model.User; import bookstore.model.result.FailureMessage; import bookstore.model.result.SuccessMessage; import bookstore.service.AppService; -import bookstore.util.PasswordUtil; import bookstore.util.StringUtil; import bookstore.util.Validator; @@ -141,12 +139,7 @@ public String updateMyProfile() { return ERROR; } - user.setUsername(getUsername()); - if (!getPassword().equals("")) - user.setPassword(PasswordUtil.passwordHash(getPassword())); - user.setNickname(getNickname()); - user.setAvatar(getAvatar()); - appService.updateUser(user); + appService.updateUser(user, getUsername(), getPassword(), getNickname(), getAvatar()); retJson = new SuccessMessage(); return SUCCESS; @@ -182,14 +175,7 @@ public String updateMyAddress() { return ERROR; } - List newAddresses = new ArrayList(); - for (String address: addressArray) { - String addr = address.trim(); - if (!addr.isEmpty()) - newAddresses.add(addr); - } - - appService.updateUserAddress(user.getId(), newAddresses); + appService.updateUserAddress(user.getId(), addressArray); retJson = new SuccessMessage(); return SUCCESS; diff --git a/src/main/java/bookstore/action/UploadImageAction.java b/src/main/java/bookstore/action/UploadImageAction.java index 03d7442..e0c4b14 100644 --- a/src/main/java/bookstore/action/UploadImageAction.java +++ b/src/main/java/bookstore/action/UploadImageAction.java @@ -1,14 +1,11 @@ package bookstore.action; import java.io.File; -import java.util.concurrent.ThreadLocalRandom; - -import org.apache.commons.io.FileUtils; import org.apache.commons.io.FilenameUtils; import bookstore.model.result.FailureMessage; import bookstore.model.result.SuccessMessage; -import bookstore.util.HashUtil; +import bookstore.service.AppService; public class UploadImageAction extends BaseAction { @@ -17,10 +14,12 @@ public class UploadImageAction extends BaseAction { private File file; private String filename; - private String uploadPath = "img/upload/"; + private final static String uploadPath = "img/upload/"; private Object retJson; + private AppService appService; + // Getters and setters public void setImage(File file) { @@ -39,6 +38,13 @@ public void setRetJson(Object retJson) { this.retJson = retJson; } + public AppService getAppService() { + return appService; + } + + public void setAppService(AppService appService) { + this.appService = appService; + } // Actions @@ -49,21 +55,17 @@ public String execute() { return LOGIN; } - try { - String uploadDir = FilenameUtils.concat(application().getRealPath("/"), uploadPath); - String newName = HashUtil.sha1File(file) + "_" - + Long.toString(System.currentTimeMillis()) + "_" - + Integer.toString(ThreadLocalRandom.current().nextInt(1, 1001)) + "." - + FilenameUtils.getExtension(filename); - File newFile = new File(uploadDir, newName); - FileUtils.copyFile(file, newFile); - retJson = new SuccessMessage(newName); - return SUCCESS; - } - catch (Exception e) { + String newFilename = appService.uploadImage(FilenameUtils.concat(application().getRealPath("/"), uploadPath), + file, filename); + + if (newFilename.isEmpty()) { retJson = new FailureMessage("上传失败！请检查文件大小和格式。"); return ERROR; } + + retJson = new SuccessMessage(newFilename); + return SUCCESS; + } } diff --git a/src/main/java/bookstore/service/AppService.java b/src/main/java/bookstore/service/AppService.java index ab98ca0..f2f1dae 100644 --- a/src/main/java/bookstore/service/AppService.java +++ b/src/main/java/bookstore/service/AppService.java @@ -1,5 +1,6 @@ package bookstore.service; +import java.io.File; import java.util.List; import bookstore.model.Book; @@ -25,10 +26,12 @@ public interface AppService { public BookDetail getBookDetailById(int id, boolean isAdmin); - public Integer addBook(Book book); + public Integer addBook(String name, String image, String author, String press, String price, String stock, String description); public void updateBook(Book book); + public void updateBook(Book book, String name, String image, String author, String press, String price, String stock, String description); + public void deleteBook(Book book); @@ -39,9 +42,9 @@ public interface AppService { public Category getCategoryById(int id); - public Integer addCategory(Category category); + public Integer addCategory(String name); - public void updateCategory(Category category); + public void updateCategory(Category category, String name); public void deleteCategory(Category category); @@ -55,7 +58,7 @@ public interface AppService { public BookCategory findBC(int categoryId, int bookId); - public Integer addBC(BookCategory bc); + public Integer addBC(int categoryId, int bookId); public void deleteBC(BookCategory bc); @@ -80,6 +83,8 @@ public interface AppService { public Object payOrder(Order order); + public Object payCart(User user); + public void deleteOrder(Order order); @@ -100,9 +105,13 @@ public interface AppService { public OrderItem getOrderItemById(int id); public Integer addOrderItem(OrderItem orderItem); + + public void addItemToCart(User user, int bookId, int quantity); public void updateOrderItem(OrderItem orderItem); + public void updateOrderItem(OrderItem orderItem, int quantity); + public void deleteOrderItem(OrderItem orderItem); @@ -119,9 +128,13 @@ public interface AppService { public boolean usernameExists(String username); - public Integer addUser(User user); + public Integer addUser(String username, String password, String nickname, String avatar, String balance, String role); public void updateUser(User user); + + public void updateUser(User user, String username, String password, String nickname, String avatar); + + public void updateUser(User user, String username, String password, String nickname, String avatar, String balance, String role); public void deleteUser(User user); @@ -137,5 +150,10 @@ public interface AppService { public List statBook(int bookId, String startDate, String endDate); public List statUser(String username, String startDate, String endDate); + + + // Upload Image + + public String uploadImage(String path, File file, String filename); } diff --git a/src/main/java/bookstore/service/impl/AppServiceImpl.java b/src/main/java/bookstore/service/impl/AppServiceImpl.java index 64f067a..ab7d490 100644 --- a/src/main/java/bookstore/service/impl/AppServiceImpl.java +++ b/src/main/java/bookstore/service/impl/AppServiceImpl.java @@ -1,6 +1,12 @@ package bookstore.service.impl; +import java.io.File; +import java.util.ArrayList; import java.util.List; +import java.util.concurrent.ThreadLocalRandom; + +import org.apache.commons.io.FileUtils; +import org.apache.commons.io.FilenameUtils; import bookstore.dao.BookCategoryDao; import bookstore.dao.BookDao; @@ -20,6 +26,8 @@ import bookstore.model.result.SuccessMessage; import bookstore.model.result.UserDetail; import bookstore.service.AppService; +import bookstore.util.HashUtil; +import bookstore.util.PasswordUtil; public class AppServiceImpl implements AppService { @@ -88,13 +96,33 @@ public BookDetail getBookDetailById(int id, boolean isAdmin) { return new BookDetail(book, bookDao.getBookCategories(id), isAdmin); } - public Integer addBook(Book book) { - return bookDao.save(book); + public Integer addBook(String name, String image, String author, String press, String price, String stock, String description) { + Book book = new Book(); + book.setName(name); + book.setImage(image); + book.setAuthor(author); + book.setPress(press); + book.setPrice((int)(Float.parseFloat(price) * 100)); + book.setStock(Integer.parseInt(stock)); + book.setDescription(description); + bookDao.save(book); + return book.getId(); } - + public void updateBook(Book book) { bookDao.update(book); } + + public void updateBook(Book book, String name, String image, String author, String press, String price, String stock, String description) { + book.setName(name); + book.setImage(image); + book.setAuthor(author); + book.setPress(press); + book.setPrice((int)(Float.parseFloat(price) * 100)); + book.setStock(Integer.parseInt(stock)); + book.setDescription(description); + bookDao.update(book); + } public void deleteBook(Book book) { bookDao.softDelete(book); @@ -112,11 +140,14 @@ public Category getCategoryById(int id) { return categoryDao.getCategoryById(id); } - public Integer addCategory(Category category) { - return categoryDao.save(category); + public Integer addCategory(String name) { + Category category = new Category(name); + categoryDao.save(category); + return category.getId(); } - public void updateCategory(Category category) { + public void updateCategory(Category category, String name) { + category.setName(name); categoryDao.update(category); } @@ -140,8 +171,8 @@ public BookCategory findBC(int categoryId, int bookId) { return bookCategoryDao.findBC(categoryId, bookId); } - public Integer addBC(BookCategory bc) { - return bookCategoryDao.save(bc); + public Integer addBC(int categoryId, int bookId) { + return bookCategoryDao.save(new BookCategory(categoryId, bookId)); } public void deleteBC(BookCategory bc) { @@ -233,6 +264,10 @@ public Object payOrder(Order order) { return new SuccessMessage(); } + public Object payCart(User user) { + return payOrder(getUserCart(user.getId())); + } + public void deleteOrder(Order order) { orderDao.softDelete(order); } @@ -270,9 +305,19 @@ public Integer addOrderItem(OrderItem orderItem) { return orderItemDao.save(orderItem); } + public void addItemToCart(User user, int bookId, int quantity) { + Order cart = getUserCart(user.getId()); + orderItemDao.save(new OrderItem(cart.getId(), bookId, quantity)); + } + public void updateOrderItem(OrderItem orderItem) { orderItemDao.update(orderItem); } + + public void updateOrderItem(OrderItem orderItem, int quantity) { + orderItem.setQuantity(quantity); + orderItemDao.update(orderItem); + } public void deleteOrderItem(OrderItem orderItem) { orderItemDao.delete(orderItem); @@ -305,16 +350,43 @@ public boolean usernameExists(String username) { return userDao.usernameExists(username); } - public Integer addUser(User user) { - int ret = userDao.save(user); + public Integer addUser(String username, String password, String nickname, String avatar, String balance, String role) { + User user = new User(); + user.setUsername(username); + user.setPassword(PasswordUtil.passwordHash(password)); + user.setNickname(nickname); + user.setAvatar(avatar); + user.setBalance((int)(Float.parseFloat(balance) * 100)); + user.setRole(role.equals("1")); + userDao.save(user); createUserCart(user.getId()); - return ret; + return user.getId(); } - + public void updateUser(User user) { userDao.update(user); } + public void updateUser(User user, String username, String password, String nickname, String avatar) { + user.setUsername(username); + if (!password.isEmpty()) + user.setPassword(PasswordUtil.passwordHash(password)); + user.setNickname(nickname); + user.setAvatar(avatar); + userDao.update(user); + } + + public void updateUser(User user, String username, String password, String nickname, String avatar, String balance, String role) { + user.setUsername(username); + if (!password.isEmpty()) + user.setPassword(PasswordUtil.passwordHash(password)); + user.setNickname(nickname); + user.setAvatar(avatar); + user.setBalance((int)(Float.parseFloat(balance) * 100)); + user.setRole(role.equals("1")); + userDao.update(user); + } + public void deleteUser(User user) { userDao.softDelete(user); } @@ -324,7 +396,13 @@ public List getUserAddress(int userId) throws Exception { } public void updateUserAddress(int userId, List addresses) { - userDao.updateUserAddress(userId, addresses); + List newAddresses = new ArrayList(); + for (String address: addresses) { + String addr = address.trim(); + if (!addr.isEmpty()) + newAddresses.add(addr); + } + userDao.updateUserAddress(userId, newAddresses); } @@ -344,4 +422,22 @@ public List statBook(int bookId, String startDate, String endDate) { public List statUser(String username, String startDate, String endDate) { return statDao.statUser(username, startDate, endDate); } + + + // Upload Image + + public String uploadImage(String path, File file, String filename) { + try { + String newName = HashUtil.sha1File(file) + "_" + + Long.toString(System.currentTimeMillis()) + "_" + + Integer.toString(ThreadLocalRandom.current().nextInt(1, 1001)) + "." + + FilenameUtils.getExtension(filename); + File newFile = new File(path, newName); + FileUtils.copyFile(file, newFile); + return newName; + } + catch (Exception e) { + return ""; + } + } } diff --git a/src/main/webapp/js/helper/message.js b/src/main/webapp/js/helper/message.js index ecc070a..fd71684 100644 --- a/src/main/webapp/js/helper/message.js +++ b/src/main/webapp/js/helper/message.js @@ -1,16 +1,16 @@ var shortProcessingText = function (text, is12) { - return '
' + text + '
'; + return '
' + escapeHtml(text) + '
'; } var shortSuccessText = function (text, is12) { - return '
' + text + '
'; + return '
' + escapeHtml(text) + '
'; } var shortFailText = function (text, is12) { - return '
' + text + '
'; + return '
' + escapeHtml(text) + '
'; } var alertFailHTML = ''; var errorAlertHTML = function (text) { - return alertFailHTML + text + alertDismissHTML; + return alertFailHTML + escapeHtml(text) + alertDismissHTML; }