Skip to content

Commit e558055

Browse files
committed
/series/add: allow to specify image URL.
1 parent 6073693 commit e558055

File tree

8 files changed

+301
-5
lines changed

8 files changed

+301
-5
lines changed

src/main/java/ru/mystamps/web/config/MvcConfig.java

+5
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646

4747
import ru.mystamps.web.Url;
4848
import ru.mystamps.web.controller.converter.LinkEntityDtoGenericConverter;
49+
import ru.mystamps.web.controller.interceptor.DownloadImageInterceptor;
4950
import ru.mystamps.web.support.spring.security.CurrentUserArgumentResolver;
5051

5152
@Configuration
@@ -128,6 +129,10 @@ public void addInterceptors(InterceptorRegistry registry) {
128129
interceptor.setParamName("lang");
129130

130131
registry.addInterceptor(interceptor);
132+
133+
// TODO: check add series with category/country
134+
registry.addInterceptor(new DownloadImageInterceptor())
135+
.addPathPatterns(Url.ADD_SERIES_PAGE);
131136
}
132137

133138
@Override
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,260 @@
1+
/*
2+
* Copyright (C) 2009-2017 Slava Semushin <[email protected]>
3+
*
4+
* This program is free software; you can redistribute it and/or modify
5+
* it under the terms of the GNU General Public License as published by
6+
* the Free Software Foundation; either version 2 of the License, or
7+
* (at your option) any later version.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU General Public License
15+
* along with this program; if not, write to the Free Software
16+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
17+
*/
18+
package ru.mystamps.web.controller.interceptor;
19+
20+
import java.io.BufferedInputStream;
21+
import java.io.ByteArrayInputStream;
22+
import java.io.File;
23+
import java.io.FileNotFoundException;
24+
import java.io.IOException;
25+
import java.io.InputStream;
26+
import java.net.HttpURLConnection;
27+
import java.net.MalformedURLException;
28+
import java.net.URL;
29+
import java.net.URLConnection;
30+
import java.util.concurrent.TimeUnit;
31+
32+
import javax.servlet.http.HttpServletRequest;
33+
import javax.servlet.http.HttpServletResponse;
34+
35+
import org.apache.commons.lang3.StringUtils;
36+
37+
import org.slf4j.Logger;
38+
import org.slf4j.LoggerFactory;
39+
40+
import org.springframework.util.StreamUtils;
41+
import org.springframework.web.multipart.MultipartFile;
42+
import org.springframework.web.multipart.support.StandardMultipartHttpServletRequest;
43+
import org.springframework.web.servlet.handler.HandlerInterceptorAdapter;
44+
45+
import lombok.RequiredArgsConstructor;
46+
47+
// TODO: javadoc
48+
public class DownloadImageInterceptor extends HandlerInterceptorAdapter {
49+
private static final Logger LOG = LoggerFactory.getLogger(DownloadImageInterceptor.class);
50+
51+
@Override
52+
@SuppressWarnings("PMD.SignatureDeclareThrowsException")
53+
public boolean preHandle(
54+
HttpServletRequest request,
55+
HttpServletResponse response,
56+
Object handler) throws Exception {
57+
58+
if (!"POST".equals(request.getMethod())) {
59+
return true;
60+
}
61+
62+
// Inspecting AddSeriesForm.imageUrl field.
63+
// If it doesn't have a value, then nothing to do here.
64+
String imageUrl = request.getParameter("imageUrl");
65+
if (StringUtils.isEmpty(imageUrl)) {
66+
return true;
67+
}
68+
69+
if (!(request instanceof StandardMultipartHttpServletRequest)) {
70+
LOG.warn(
71+
"Unknown type of request ({}). "
72+
+ "Downloading images from external servers won't work!",
73+
request
74+
);
75+
return true;
76+
}
77+
78+
LOG.debug("preHandle imageUrl = {}", imageUrl);
79+
80+
StandardMultipartHttpServletRequest multipartRequest =
81+
(StandardMultipartHttpServletRequest)request;
82+
MultipartFile image = multipartRequest.getFile("image");
83+
if (image != null && StringUtils.isNotEmpty(image.getOriginalFilename())) {
84+
LOG.debug("User provided image, exited");
85+
// user specified both image and image URL, we'll handle it later, during validation
86+
return true;
87+
}
88+
89+
// user specified image URL: we should download file and represent it as "image" field.
90+
// Doing this our validation will be able to check downloaded file later.
91+
92+
// TODO: where/how to show possible errors during downloading?
93+
byte[] data;
94+
try {
95+
URL url = new URL(imageUrl);
96+
LOG.debug("URL.getPath(): {} / URL.getFile(): {}", url.getPath(), url.getFile());
97+
98+
// TODO: add title to field that only HTTP protocol is supported
99+
// and no redirects are allowed
100+
if (!"http".equals(url.getProtocol())) {
101+
// TODO(security): fix possible log injection
102+
LOG.info("Invalid link '{}': only HTTP protocol is supported", imageUrl);
103+
return true;
104+
}
105+
106+
try {
107+
URLConnection connection = url.openConnection();
108+
if (!(connection instanceof HttpURLConnection)) {
109+
LOG.warn(
110+
"Unknown type of connection class ({}). "
111+
+ "Downloading images from external servers won't work!",
112+
connection
113+
);
114+
return true;
115+
}
116+
HttpURLConnection conn = (HttpURLConnection)connection;
117+
118+
conn.setRequestProperty(
119+
"User-Agent",
120+
"Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0"
121+
);
122+
123+
long timeout = TimeUnit.SECONDS.toMillis(1);
124+
conn.setReadTimeout(Math.toIntExact(timeout));
125+
LOG.debug("getReadTimeout(): {}", conn.getReadTimeout());
126+
127+
// TODO: how bad is it?
128+
conn.setInstanceFollowRedirects(false);
129+
130+
try {
131+
conn.connect();
132+
} catch (IOException ex) {
133+
// TODO(security): fix possible log injection
134+
LOG.error("Couldn't connect to '{}': {}", imageUrl, ex.getMessage());
135+
return true;
136+
}
137+
138+
try (InputStream stream = new BufferedInputStream(conn.getInputStream())) {
139+
int status = conn.getResponseCode();
140+
if (status != HttpURLConnection.HTTP_OK) {
141+
// TODO(security): fix possible log injection
142+
LOG.error(
143+
"Couldn't download file '{}': bad response status {}",
144+
imageUrl,
145+
status
146+
);
147+
return true;
148+
}
149+
150+
// TODO: add protection against huge files
151+
int contentLength = conn.getContentLength();
152+
LOG.debug("Content-Length: {}", contentLength);
153+
if (contentLength <= 0) {
154+
// TODO(security): fix possible log injection
155+
LOG.error(
156+
"Couldn't download file '{}': it has {} bytes length",
157+
imageUrl,
158+
contentLength
159+
);
160+
return true;
161+
}
162+
163+
String contentType = conn.getContentType();
164+
LOG.debug("Content-Type: {}", contentType);
165+
if (!"image/jpeg".equals(contentType) && !"image/png".equals(contentType)) {
166+
// TODO(security): fix possible log injection
167+
LOG.error(
168+
"Couldn't download file '{}': unsupported image type '{}'",
169+
imageUrl,
170+
contentType
171+
);
172+
return true;
173+
}
174+
175+
data = StreamUtils.copyToByteArray(stream);
176+
177+
} catch (FileNotFoundException ignored) {
178+
// TODO: show error to user
179+
// TODO(security): fix possible log injection
180+
LOG.error("Couldn't download file '{}': not found", imageUrl);
181+
return true;
182+
183+
} catch (IOException ex) {
184+
// TODO(security): fix possible log injection
185+
LOG.error(
186+
"Couldn't download file from URL '{}': {}",
187+
imageUrl,
188+
ex.getMessage()
189+
);
190+
return true;
191+
}
192+
193+
} catch (IOException ex) {
194+
LOG.error("Couldn't open connection: {}", ex.getMessage());
195+
return true;
196+
}
197+
198+
} catch (MalformedURLException ex) {
199+
// TODO(security): fix possible log injection
200+
// TODO: show error to user
201+
LOG.error("Invalid image URL '{}': {}", imageUrl, ex.getMessage());
202+
return true;
203+
}
204+
205+
// TODO: use URL.getFile() instead of full link?
206+
multipartRequest.getMultiFileMap().set("image", new MyMultipartFile(data, imageUrl));
207+
208+
// TODO: how we can validate url?
209+
210+
return true;
211+
}
212+
213+
@RequiredArgsConstructor
214+
private static class MyMultipartFile implements MultipartFile {
215+
private final byte[] content;
216+
private final String link;
217+
218+
@Override
219+
public String getName() {
220+
throw new IllegalStateException("Not implemented");
221+
}
222+
223+
@Override
224+
public String getOriginalFilename() {
225+
return link;
226+
}
227+
228+
// TODO: preserve original content type
229+
@Override
230+
public String getContentType() {
231+
return "image/jpeg";
232+
}
233+
234+
@Override
235+
public boolean isEmpty() {
236+
return getSize() == 0;
237+
}
238+
239+
@Override
240+
public long getSize() {
241+
return content.length;
242+
}
243+
244+
@Override
245+
public byte[] getBytes() throws IOException {
246+
return content;
247+
}
248+
249+
@Override
250+
public InputStream getInputStream() throws IOException {
251+
return new ByteArrayInputStream(content);
252+
}
253+
254+
@Override
255+
public void transferTo(File dest) throws IOException, IllegalStateException {
256+
throw new IllegalStateException("Not implemented");
257+
}
258+
}
259+
260+
}

src/main/java/ru/mystamps/web/model/AddSeriesForm.java

+9-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import javax.validation.constraints.Size;
2727

2828
import org.hibernate.validator.constraints.Range;
29+
import org.hibernate.validator.constraints.URL;
2930

3031
import org.springframework.web.multipart.MultipartFile;
3132

@@ -56,6 +57,9 @@
5657

5758
@Getter
5859
@Setter
60+
// TODO: image or imageUrl must be specified (but not both)
61+
// TODO: localize URL
62+
// TODO: disallow urls like http://test
5963
// TODO: combine price with currency to separate class
6064
@SuppressWarnings({"PMD.TooManyFields", "PMD.AvoidDuplicateLiterals"})
6165
@NotNullIfFirstField.List({
@@ -128,13 +132,17 @@ public class AddSeriesForm implements AddSeriesDto {
128132
@Size(max = MAX_SERIES_COMMENT_LENGTH, message = "{value.too-long}")
129133
private String comment;
130134

131-
@NotNull
135+
// @NotNull
132136
@NotEmptyFilename(groups = Image1Checks.class)
133137
@NotEmptyFile(groups = Image2Checks.class)
134138
@MaxFileSize(value = MAX_IMAGE_SIZE, unit = Unit.Kbytes, groups = Image3Checks.class)
135139
@ImageFile(groups = Image3Checks.class)
136140
private MultipartFile image;
137141

142+
// Name of this field must match with the field name that
143+
// is being inspected by DownloadImageInterceptor
144+
@URL(protocol = "http")
145+
private String imageUrl;
138146

139147
@GroupSequence({
140148
ReleaseDate1Checks.class,

src/main/java/ru/mystamps/web/support/togglz/Features.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,11 @@ public enum Features implements Feature {
8282

8383
@Label("/series/add: show link with auto-suggestions")
8484
@EnabledByDefault
85-
SHOW_SUGGESTION_LINK;
85+
SHOW_SUGGESTION_LINK,
86+
87+
@Label("/series/add: possibility to download image from external server")
88+
@EnabledByDefault
89+
DOWNLOAD_IMAGE;
8690

8791
public boolean isActive() {
8892
return FeatureContext.getFeatureManager().isActive(this);

src/main/resources/ru/mystamps/i18n/Messages.properties

+1
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ t_sg = Gibbons
117117
t_add_comment = Add comment
118118
t_comment = Comment
119119
t_image = Image
120+
t_image_url = Image URL
120121
t_add_more_images_hint = Later you will be able to add additional images
121122
t_not_chosen = Not chosen
122123
t_create_category_hint = You can also <a tabindex="-1" href="{0}">add a new category</a>

src/main/resources/ru/mystamps/i18n/Messages_ru.properties

+1
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ t_sg = Gibbons
117117
t_add_comment = Добавить комментарий
118118
t_comment = Комментарий
119119
t_image = Изображение
120+
t_image_url = Ссылка на изображение
120121
t_add_more_images_hint = Вы сможете добавить дополнительные изображения позже
121122
t_not_chosen = Не выбрана
122123
t_create_category_hint = Вы также можете <a tabindex="-1" href="{0}">добавить новую категорию</a>

src/main/webapp/WEB-INF/views/series/add.html

+19-2
Original file line numberDiff line numberDiff line change
@@ -262,10 +262,15 @@ <h3 th:text="${#strings.capitalize(add_series)}">
262262
<span class="field-label" th:text="#{t_image}">
263263
Image
264264
</span>
265-
<span id="image.required" class="required_field">*</span>
265+
<!--/*/
266+
<span id="image.required" class="required_field" togglz:inactive="DOWNLOAD_IMAGE">*</span>
267+
/*/-->
266268
</label>
267269
<div class="col-sm-7">
268-
<input type="file" id="image" style="box-shadow: none; border: 0px;" required="required" accept="image/png,image/jpeg" th:field="*{image}" />
270+
<input type="file" id="image" style="box-shadow: none; border: 0px;" accept="image/png,image/jpeg" th:field="*{image}" togglz:active="DOWNLOAD_IMAGE"/>
271+
<!--/*/
272+
<input type="file" id="image" style="box-shadow: none; border: 0px;" required="required" accept="image/png,image/jpeg" th:field="*{image}" togglz:inactive="DOWNLOAD_IMAGE"/>
273+
/*/-->
269274
<small togglz:active="ADD_ADDITIONAL_IMAGES_TO_SERIES" sec:authorize="hasAuthority('ADD_IMAGES_TO_SERIES')">
270275
<span class="hint-block" th:text="#{t_add_more_images_hint}">
271276
Later you will be able to add additional images
@@ -277,6 +282,18 @@ <h3 th:text="${#strings.capitalize(add_series)}">
277282
</div>
278283
</div>
279284

285+
<div class="form-group form-group-sm" th:classappend="${#fields.hasErrors('imageUrl') ? 'has-error' : ''}" togglz:active="DOWNLOAD_IMAGE">
286+
<label for="image-url" class="control-label col-sm-3">
287+
<span class="field-label" th:text="#{t_image_url}">
288+
Image URL
289+
</span>
290+
</label>
291+
<div class="col-sm-5">
292+
<input type="url" id="image-url" class="form-control" th:field="*{imageUrl}" />
293+
<span id="image-url.errors" class="help-block" th:if="${#fields.hasErrors('imageUrl')}" th:each="error : ${#fields.errors('imageUrl')}" th:text="${error}"></span>
294+
</div>
295+
</div>
296+
280297
<div class="form-group js-collapse-toggle-header">
281298
<div class="col-sm-offset-3 col-sm-5">
282299
<span class="glyphicon glyphicon-chevron-right" th:class="${issueDateHasErrors or issueDateHasValues ? 'glyphicon glyphicon-chevron-down' : 'glyphicon glyphicon-chevron-right'}"></span>&nbsp;<a href="javascript:void(0)" id="specify-issue-date-link" data-toggle="collapse" data-target=".js-issue-date" th:text="#{t_specify_issue_date}">Specify date of release</a>

src/test/java/ru/mystamps/web/tests/page/AddSeriesPage.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public AddSeriesPage(WebDriver driver) {
7979
inputField("gibbonsNumbers").withLabel(tr("t_sg")),
8080
inputField("gibbonsPrice"),
8181
textareaField("comment").withLabel(tr("t_comment")).accessibleByAll(false),
82-
required(uploadFileField("image").withLabel(tr("t_image")))
82+
uploadFileField("image").withLabel(tr("t_image"))
8383
)
8484
.and()
8585
.with(submitButton(tr("t_add")))

0 commit comments

Comments
 (0)