From 3e87cc6a73fbfc165f81d5f7a3d7d1f76e7feb9a Mon Sep 17 00:00:00 2001 From: Vitaly Takmazov Date: Wed, 1 Nov 2017 01:57:08 +0300 Subject: www: /post should not throw if img or attach is present --- .../main/java/com/juick/database/MySqlUpdater.java | 3 +- .../java/com/juick/server/util/ImageUtils.java | 4 +- .../juick/www/configuration/WebSecurityConfig.java | 2 +- .../java/com/juick/www/controllers/Messages.java | 15 ++++-- .../java/com/juick/www/controllers/NewMessage.java | 16 ++---- juick-www/src/main/static/scripts.js | 4 +- .../src/test/java/com/juick/www/WebAppTests.java | 62 ++++++++++++++++++++-- 7 files changed, 81 insertions(+), 25 deletions(-) diff --git a/juick-server-jdbc/src/main/java/com/juick/database/MySqlUpdater.java b/juick-server-jdbc/src/main/java/com/juick/database/MySqlUpdater.java index 3d425648..69cde40b 100644 --- a/juick-server-jdbc/src/main/java/com/juick/database/MySqlUpdater.java +++ b/juick-server-jdbc/src/main/java/com/juick/database/MySqlUpdater.java @@ -17,6 +17,7 @@ package com.juick.database; +import org.apache.commons.codec.CharEncoding; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; @@ -68,7 +69,7 @@ public class MySqlUpdater { InputStream is = Thread.currentThread().getContextClassLoader().getResourceAsStream(updateSqlResource); ) { if (is != null) { - String content = IOUtils.toString(is, StandardCharsets.UTF_8); + String content = IOUtils.toString(is, CharEncoding.UTF_8); if (StringUtils.isNotEmpty(content)) { String[] sqlArray = content.split(";"); if (sqlArray.length > 0) { diff --git a/juick-server-web/src/main/java/com/juick/server/util/ImageUtils.java b/juick-server-web/src/main/java/com/juick/server/util/ImageUtils.java index 7f21c0a5..4403abc0 100644 --- a/juick-server-web/src/main/java/com/juick/server/util/ImageUtils.java +++ b/juick-server-web/src/main/java/com/juick/server/util/ImageUtils.java @@ -19,20 +19,20 @@ package com.juick.server.util; import org.apache.commons.imaging.ImageInfo; -import org.apache.commons.io.FilenameUtils; import org.apache.commons.imaging.ImageReadException; import org.apache.commons.imaging.Imaging; import org.apache.commons.imaging.common.ImageMetadata; import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata; import org.apache.commons.imaging.formats.tiff.TiffField; import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants; +import org.apache.commons.io.FilenameUtils; import org.imgscalr.Scalr; import org.imgscalr.Scalr.Rotation; import javax.imageio.ImageIO; import java.awt.image.BufferedImage; -import java.io.IOException; import java.io.File; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; diff --git a/juick-www/src/main/java/com/juick/www/configuration/WebSecurityConfig.java b/juick-www/src/main/java/com/juick/www/configuration/WebSecurityConfig.java index 3dc7bc76..231c6e2e 100644 --- a/juick-www/src/main/java/com/juick/www/configuration/WebSecurityConfig.java +++ b/juick-www/src/main/java/com/juick/www/configuration/WebSecurityConfig.java @@ -72,7 +72,7 @@ public class WebSecurityConfig extends WebSecurityConfigurerAdapter { http.addFilterAfter(hashParamAuthenticationFilter(), BasicAuthenticationFilter.class); http .authorizeRequests() - .antMatchers("/settings", "/pm/**", "/**/bl", "/_twitter").authenticated() + .antMatchers("/settings", "/pm/**", "/**/bl", "/_twitter", "/post", "/comment").authenticated() .anyRequest().permitAll() .and() .anonymous().principal(JuickUser.ANONYMOUS_USER).authorities(JuickUser.ANONYMOUS_AUTHORITY) diff --git a/juick-www/src/main/java/com/juick/www/controllers/Messages.java b/juick-www/src/main/java/com/juick/www/controllers/Messages.java index 7c96705f..75e81b94 100644 --- a/juick-www/src/main/java/com/juick/www/controllers/Messages.java +++ b/juick-www/src/main/java/com/juick/www/controllers/Messages.java @@ -542,8 +542,16 @@ public class Messages { headers += ""; } String cardType = StringUtils.isNotEmpty(msg.getAttachmentType()) ? "summary_large_image" : "summary"; - String msgImage = StringUtils.isNotEmpty(msg.getAttachmentType()) ? msg.getAttachment().getMedium().getUrl() - : "https://i.juick.com/a/" + msg.getUser().getUid() + ".png"; + if (StringUtils.isNotEmpty(msg.getAttachmentType())) { + // additional check in case of broken images + if (msg.getAttachment() != null) { + String msgImage = msg.getAttachment().getMedium().getUrl(); + headers += ""; + } + } else { + String msgImage ="https://i.juick.com/a/" + msg.getUser().getUid() + ".png"; + headers += ""; + } model.addAttribute("ogtype", "article"); String cardDescription = StringEscapeUtils.escapeHtml4(PlainTextFormatter.formatTwitterCard(msg)); headers += "\n" + @@ -551,8 +559,7 @@ public class Messages { "\n" + "\n" + "\n" + - "\n" + - ""; + "\n"; String twitterName = crosspostService.getTwitterName(msg.getUser().getUid()); if (StringUtils.isNotEmpty(twitterName)) { headers += "\n"; diff --git a/juick-www/src/main/java/com/juick/www/controllers/NewMessage.java b/juick-www/src/main/java/com/juick/www/controllers/NewMessage.java index 2de6a2ad..2b411523 100644 --- a/juick-www/src/main/java/com/juick/www/controllers/NewMessage.java +++ b/juick-www/src/main/java/com/juick/www/controllers/NewMessage.java @@ -91,14 +91,11 @@ public class NewMessage { @PostMapping("/post") public String postResult(@RequestParam(required = false) String img, - @RequestParam String body, + @RequestParam(required = false, defaultValue = StringUtils.EMPTY) String body, @RequestParam(required = false, name = "tags") String tagsStr, @RequestParam(required = false) MultipartFile attach, ModelMap model) throws IOException { com.juick.User visitor = UserUtils.getCurrentUser(); - if (visitor.getUid() == 0) { - throw new HttpForbiddenException(); - } - if (body == null || body.length() < 1 || body.length() > 4096) { + if ((StringUtils.isEmpty(body) || body.length() > 4096) && StringUtils.isEmpty(img) && attach == null) { throw new HttpBadRequestException(); } body = body.replace("\r", StringUtils.EMPTY); @@ -107,7 +104,7 @@ public class NewMessage { String attachmentFName = HttpUtils.receiveMultiPartFile(attach, webApp.getTmpDir()); - if (StringUtils.isBlank(attachmentFName) && img != null && img.length() > 10) { + if (StringUtils.isBlank(attachmentFName) && StringUtils.isNotBlank(img)) { try { URL imgUrl = new URL(img); attachmentFName = HttpUtils.downloadImage(imgUrl, webApp.getTmpDir()); @@ -200,13 +197,10 @@ public class NewMessage { public String doPostComment( @RequestParam(required = false, defaultValue = "0") Integer mid, @RequestParam(required = false, defaultValue = "0") Integer rid, - @RequestParam String body, + @RequestParam(required = false, defaultValue = StringUtils.EMPTY) String body, @RequestParam(required = false) String img, @RequestParam(required = false) MultipartFile attach) throws IOException { com.juick.User visitor = UserUtils.getCurrentUser(); - if (visitor.getUid() == 0) { - throw new HttpForbiddenException(); - } if (mid == 0) { throw new HttpBadRequestException(); } @@ -223,7 +217,7 @@ public class NewMessage { } } - if (body.length() < 1 || body.length() > 4096) { + if ((StringUtils.isEmpty(body) || body.length() > 4096) && StringUtils.isEmpty(img)) { throw new HttpBadRequestException(); } body = body.replace("\r", StringUtils.EMPTY); diff --git a/juick-www/src/main/static/scripts.js b/juick-www/src/main/static/scripts.js index 7dc24dd2..e33d1929 100644 --- a/juick-www/src/main/static/scripts.js +++ b/juick-www/src/main/static/scripts.js @@ -292,7 +292,9 @@ function newMessage(evt) { document.querySelectorAll('#newmessage .dialogtxt').forEach(t => { t.remove(); }); - if (document.querySelector('#newmessage textarea').value.length == 0) { + if (document.querySelector('#newmessage textarea').value.length == 0 + && document.querySelector('#newmessage .img').value.length == 0 + && !document.querySelector('#newmessage input[type="file"]')) { document.querySelector('#newmessage').insertAdjacentHTML('afterbegin', `

${i18n('postForm.pleaseInputMessageText')}

`); evt.preventDefault(); } diff --git a/juick-www/src/test/java/com/juick/www/WebAppTests.java b/juick-www/src/test/java/com/juick/www/WebAppTests.java index 73750cb7..48f465aa 100644 --- a/juick-www/src/test/java/com/juick/www/WebAppTests.java +++ b/juick-www/src/test/java/com/juick/www/WebAppTests.java @@ -32,6 +32,7 @@ import com.juick.service.MockImagesService; import com.juick.service.UserService; import com.juick.util.MessageUtils; import com.juick.www.configuration.SapeConfiguration; +import com.juick.www.configuration.WebSecurityConfig; import com.juick.www.configuration.WwwAppConfiguration; import com.juick.www.configuration.WwwServletConfiguration; import com.mitchellbosecke.pebble.PebbleEngine; @@ -44,15 +45,20 @@ import org.junit.runner.RunWith; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; +import org.springframework.core.io.ClassPathResource; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.mock.web.MockMultipartFile; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.htmlunit.MockMvcWebClientBuilder; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.context.WebApplicationContext; -import org.springframework.web.servlet.config.annotation.EnableWebMvc; import javax.inject.Inject; +import java.io.FileInputStream; import java.io.IOException; import java.io.StringWriter; import java.io.Writer; @@ -63,6 +69,11 @@ import java.util.stream.StreamSupport; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.startsWith; +import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; /** * Created by vitalyster on 12.01.2017. @@ -71,11 +82,10 @@ import static org.hamcrest.Matchers.startsWith; @WebAppConfiguration @ContextConfiguration(classes = { WwwServletConfiguration.class, WwwAppConfiguration.class, SapeConfiguration.class, - RepositoryConfiguration.class + RepositoryConfiguration.class, WebSecurityConfig.class }) public class WebAppTests { @Configuration - @EnableWebMvc @ComponentScan(basePackages = "com.juick.www.controllers") static class Config { @Bean @@ -89,6 +99,7 @@ public class WebAppTests { @Inject private WebApp webApp; + private static MockMvc mockMvc; private static WebClient webClient; @Inject @@ -109,7 +120,10 @@ public class WebAppTests { @Before public void setup() { if (!isSetUp) { - webClient = MockMvcWebClientBuilder.webAppContextSetup(this.wac).build(); + mockMvc = MockMvcBuilders.webAppContextSetup(wac) + .apply(springSecurity()) + .build(); + webClient = MockMvcWebClientBuilder.mockMvcSetup(mockMvc).build(); webClient.getOptions().setJavaScriptEnabled(false); webClient.getOptions().setCssEnabled(false); webClient.getOptions().setThrowExceptionOnFailingStatusCode(false); @@ -196,6 +210,44 @@ public class WebAppTests { assertThat(fetchMeta(String.format("http://localhost:8080/ugnich/%d", mid2), "og:description") .getAttribute("content"), startsWith(StringEscapeUtils.escapeHtml4(MessageUtils.getMessageHashTags(message)))); - + } + @Test + public void postMessageTests() throws Exception { + mockMvc.perform(post("/post").param("body", "yo")).andExpect(redirectedUrl("http://localhost/login")); + MvcResult loginResult = mockMvc.perform(post("/login") + .param("username", ugnichName) + .param("password", ugnichPassword)).andReturn(); + mockMvc.perform(post("/post") + .cookie(loginResult.getResponse().getCookies()) + .param("wrong_param", "yo")).andExpect(status().isBadRequest()); + mockMvc.perform(post("/post") + .cookie(loginResult.getResponse().getCookies()) + .param("body", "yo")).andExpect(status().isOk()); + mockMvc.perform(post("/post") + .cookie(loginResult.getResponse().getCookies()) + .param("img", "http://static.juick.com/settings/facebook.png")).andExpect(status().isOk()); + mockMvc.perform(post("/post") + .cookie(loginResult.getResponse().getCookies()) + .param("img", "bad_url")).andExpect(status().isBadRequest()); + FileInputStream fi = new FileInputStream(new ClassPathResource("tagscloud.png").getFile()); + MockMultipartFile file = new MockMultipartFile("attach", fi); + mockMvc.perform(multipart("/post") + .file(file) + .cookie(loginResult.getResponse().getCookies())).andExpect(status().isOk()); + int mid = messagesService.createMessage(ugnich.getUid(), "dummy message", null, null); + mockMvc.perform(post("/comment") + .param("mid", String.valueOf(mid)) + .param("body", "yo")).andExpect(redirectedUrl("http://localhost/login")); + mockMvc.perform(post("/comment") + .cookie(loginResult.getResponse().getCookies()) + .param("wrong_param", "yo")).andExpect(status().isBadRequest()); + mockMvc.perform(post("/comment") + .cookie(loginResult.getResponse().getCookies()) + .param("mid", String.valueOf(mid)) + .param("wrong_param", "yo")).andExpect(status().isBadRequest()); + mockMvc.perform(post("/comment") + .cookie(loginResult.getResponse().getCookies()) + .param("mid", String.valueOf(mid)) + .param("body", "yo")).andExpect(redirectedUrl(String.format("/%s/%d#%d", ugnichName, mid, 1))); } } -- cgit v1.2.3