Skip to content

Commit

Permalink
favor req.file(name) over req.param(name).toUpload fix #819
Browse files Browse the repository at this point in the history
Optional Upload with malformed file multipart part fix #803
  • Loading branch information
jknack committed Jun 29, 2017
1 parent 1b8eab1 commit bdb4177
Show file tree
Hide file tree
Showing 22 changed files with 143 additions and 169 deletions.
1 change: 0 additions & 1 deletion jooby/src/main/java/org/jooby/Jooby.java
Original file line number Diff line number Diff line change
Expand Up @@ -2854,7 +2854,6 @@ private Injector bootstrap(final Config args,
parsers.addBinding().toInstance(BuiltinParser.Collection);
parsers.addBinding().toInstance(BuiltinParser.Optional);
parsers.addBinding().toInstance(BuiltinParser.Enum);
parsers.addBinding().toInstance(BuiltinParser.Upload);
parsers.addBinding().toInstance(BuiltinParser.Bytes);

/** basic render */
Expand Down
15 changes: 2 additions & 13 deletions jooby/src/main/java/org/jooby/Mutant.java
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,7 @@ default SortedSet<String> toSortedSet() {
@SuppressWarnings("unchecked")
default <T extends Comparable<T>> SortedSet<T> toSortedSet(final Class<T> type) {
return (SortedSet<T>) to(TypeLiteral.get(
Types.newParameterizedType(SortedSet.class, Primitives.wrap(type))
));
Types.newParameterizedType(SortedSet.class, Primitives.wrap(type))));
}

/**
Expand All @@ -468,17 +467,7 @@ default Optional<String> toOptional() {
@SuppressWarnings("unchecked")
default <T> Optional<T> toOptional(final Class<T> type) {
return (Optional<T>) to(TypeLiteral.get(
Types.newParameterizedType(Optional.class, Primitives.wrap(type))
));
}

/**
* Convert a form field to file {@link Upload}.
*
* @return A file {@link Upload}.
*/
default Upload toUpload() {
return to(Upload.class);
Types.newParameterizedType(Optional.class, Primitives.wrap(type))));
}

/**
Expand Down
19 changes: 0 additions & 19 deletions jooby/src/main/java/org/jooby/Parser.java
Original file line number Diff line number Diff line change
Expand Up @@ -466,25 +466,6 @@ interface Builder {
*/
Builder ifparams(Callback<Map<String, Mutant>> callback);

/**
* Add a HTTP upload callback. The Callback will be executed when current context is bound to a
* HTTP upload via {@link Request#param(String)}.
*
* If current {@link Context} isn't a HTTP upload a call to {@link Context#next()} is made.
*
* @param callback A upload parser callback.
* @return This builder.
*/
Builder upload(Callback<ParamReference<Upload>> callback);

/**
* Like {@link #upload(Callback)} but it skip the callback if the requested type is an
* {@link Optional}.
*
* @param callback A upload parser callback.
* @return This builder.
*/
Builder ifupload(Callback<ParamReference<Upload>> callback);
}

/**
Expand Down
32 changes: 25 additions & 7 deletions jooby/src/main/java/org/jooby/Request.java
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@

import static java.util.Objects.requireNonNull;

import java.io.IOException;
import java.nio.charset.Charset;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -417,12 +418,12 @@ public Mutant param(final String name, final String... xss) {
}

@Override
public Upload file(final String name) {
public Upload file(final String name) throws IOException {
return req.file(name);
}

@Override
public List<Upload> files(final String name) {
public List<Upload> files(final String name) throws IOException {
return req.files(name);
}

Expand Down Expand Up @@ -1013,9 +1014,27 @@ default <T> T form(final Class<T> type, final String... xss) {
*
* @param name File's name.
* @return An {@link Upload}.
* @throws IOException
*/
default Upload file(final String name) {
return param(name).toUpload();
default Upload file(final String name) throws IOException {
List<Upload> files = files(name);
if (files.size() == 0) {
throw new Err.Missing(name);
}
return files.get(0);
}

/**
* Get a file {@link Upload} with the given name or empty. The request must be a POST with
* <code>multipart/form-data</code> content-type.
*
* @param name File's name.
* @return An {@link Upload}.
* @throws IOException
*/
default Optional<Upload> ifFile(final String name) throws IOException {
List<Upload> files = files(name);
return files.size() == 0 ? Optional.empty() : Optional.of(files.get(0));
}

/**
Expand All @@ -1024,10 +1043,9 @@ default Upload file(final String name) {
*
* @param name File's name.
* @return A list of {@link Upload}.
* @throws IOException
*/
default List<Upload> files(final String name) {
return param(name).toList(Upload.class);
}
List<Upload> files(final String name) throws IOException;

/**
* Get a HTTP header.
Expand Down
21 changes: 0 additions & 21 deletions jooby/src/main/java/org/jooby/internal/BuiltinParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@
import java.util.function.Supplier;

import org.jooby.Parser;
import org.jooby.Upload;

import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -323,13 +322,6 @@ public Object parse(final TypeLiteral<?> type, final Parser.Context ctx) throws
builder.add(ctx.next(paramType, value));
}
return builder.build();
}).upload(uploads -> {
ImmutableCollection.Builder builder = parsers.get(type.getRawType()).get();
TypeLiteral<Upload> paramType = TypeLiteral.get(Upload.class);
for (Upload upload : uploads) {
builder.add(ctx.next(paramType, upload));
}
return builder.build();
});
} else {
return ctx.next();
Expand Down Expand Up @@ -359,8 +351,6 @@ public Object parse(final TypeLiteral<?> type, final Parser.Context ctx)
return java.util.Optional.empty();
}
return java.util.Optional.of(ctx.next(paramType));
}).upload(files -> {
return java.util.Optional.of(ctx.next(paramType));
});
} else {
return ctx.next();
Expand Down Expand Up @@ -391,17 +381,6 @@ Object toEnum(final Class type, final String value) {
}
},

Upload {
@Override
public Object parse(final TypeLiteral<?> type, final Context ctx) throws Throwable {
if (Upload.class == type.getRawType()) {
return ctx.upload(uploads -> uploads.get(0));
} else {
return ctx.next();
}
}
},

Bytes {
@Override
public Object parse(final TypeLiteral<?> type, final Parser.Context ctx) throws Throwable {
Expand Down
30 changes: 14 additions & 16 deletions jooby/src/main/java/org/jooby/internal/RequestImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@
import static java.util.Objects.requireNonNull;

import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -392,27 +393,24 @@ public Mutant param(final String name) {
return _param(name, null);
}

@Override
public List<Upload> files(final String name) throws IOException {
List<NativeUpload> files = req.files(name);
List<Upload> uploads = files.stream()
.map(upload -> new UploadImpl(injector, upload))
.collect(Collectors.toList());
return uploads;
}

private Mutant _param(final String name, final Function<String, String> xss) {
Mutant param = this.params.get(name);
if (param == null) {
List<NativeUpload> files = Try.of(() -> req.files(name)).getOrElseThrow(
ex -> new Err(Status.BAD_REQUEST, "Upload " + name + " resulted in error", ex));
if (files.size() > 0) {
List<Upload> uploads = files.stream()
.map(upload -> new UploadImpl(injector, upload))
.collect(Collectors.toList());
param = new MutantImpl(require(ParserExecutor.class), type(),
new UploadParamReferenceImpl(name, uploads));
StrParamReferenceImpl paramref = new StrParamReferenceImpl("parameter", name,
params(name, xss));
param = new MutantImpl(require(ParserExecutor.class), paramref);

if (paramref.size() > 0) {
this.params.put(name, param);
} else {
StrParamReferenceImpl paramref = new StrParamReferenceImpl("parameter", name,
params(name, xss));
param = new MutantImpl(require(ParserExecutor.class), paramref);

if (paramref.size() > 0) {
this.params.put(name, param);
}
}
}
return param;
Expand Down
17 changes: 16 additions & 1 deletion jooby/src/main/java/org/jooby/internal/mvc/RequestParam.java
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Parameter;
import java.lang.reflect.Type;
import java.util.List;
import java.util.Map;
import java.util.Optional;

Expand All @@ -218,6 +219,7 @@
import org.jooby.Response;
import org.jooby.Route;
import org.jooby.Session;
import org.jooby.Upload;
import org.jooby.mvc.Body;
import org.jooby.mvc.Flash;
import org.jooby.mvc.Header;
Expand Down Expand Up @@ -272,6 +274,19 @@ private interface GetValue {
builder.put(TypeLiteral.get(Session.class), (req, rsp, param) -> req.session());
builder.put(TypeLiteral.get(Types.newParameterizedType(Optional.class, Session.class)),
(req, rsp, param) -> req.ifSession());

/**
* Files
*/
builder.put(TypeLiteral.get(Upload.class), (req, rsp, param) -> req.file(param.name));
builder.put(TypeLiteral.get(Types.newParameterizedType(Optional.class, Upload.class)),
(req, rsp, param) -> {
List<Upload> files = req.files(param.name);
return files.size() == 0 ? Optional.empty() : Optional.of(files.get(0));
});
builder.put(TypeLiteral.get(Types.newParameterizedType(List.class, Upload.class)),
(req, rsp, param) -> req.files(param.name));

/**
* Cookie
*/
Expand Down Expand Up @@ -307,7 +322,7 @@ private interface GetValue {
if (param.type.getRawType() == Map.class) {
return req.flash();
}
return param.optional? req.ifFlash(param.name) : req.flash(param.name);
return param.optional ? req.ifFlash(param.name) : req.flash(param.name);
});

injector = builder.build();
Expand Down
13 changes: 0 additions & 13 deletions jooby/src/main/java/org/jooby/internal/parser/ParserBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,9 @@
import org.jooby.Parser;
import org.jooby.Parser.Builder;
import org.jooby.Parser.Callback;
import org.jooby.Upload;
import org.jooby.internal.BodyReferenceImpl;
import org.jooby.internal.EmptyBodyReference;
import org.jooby.internal.StrParamReferenceImpl;
import org.jooby.internal.UploadParamReferenceImpl;

import com.google.common.collect.ImmutableMap;
import com.google.inject.TypeLiteral;
Expand Down Expand Up @@ -280,17 +278,6 @@ public Builder ifparams(final Callback<Map<String, Mutant>> callback) {
return params(callback);
}

@Override
public Builder upload(final Callback<Parser.ParamReference<Upload>> callback) {
strategies.put(TypeLiteral.get(UploadParamReferenceImpl.class), callback);
return this;
}

@Override
public Builder ifupload(final Callback<Parser.ParamReference<Upload>> callback) {
return upload(callback);
}

@SuppressWarnings("unchecked")
public Object parse() throws Throwable {
Map<TypeLiteral<?>, Callback> map = strategies.build();
Expand Down
15 changes: 0 additions & 15 deletions jooby/src/main/java/org/jooby/internal/parser/ParserExecutor.java
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,8 @@
import org.jooby.Parser.Callback;
import org.jooby.Parser.ParamReference;
import org.jooby.Status;
import org.jooby.Upload;
import org.jooby.internal.StatusCodeProvider;
import org.jooby.internal.StrParamReferenceImpl;
import org.jooby.internal.UploadParamReferenceImpl;

import com.google.common.collect.ImmutableList;
import com.google.inject.Injector;
Expand Down Expand Up @@ -285,16 +283,6 @@ public Builder ifbody(final Callback<BodyReference> callback) {
return builder.ifbody(callback);
}

@Override
public Builder upload(final Callback<Parser.ParamReference<Upload>> callback) {
return builder.upload(callback);
}

@Override
public Builder ifupload(final Callback<ParamReference<Upload>> callback) {
return builder.ifupload(callback);
}

@Override
public Builder param(final Callback<ParamReference<String>> callback) {
return builder.param(callback);
Expand Down Expand Up @@ -356,9 +344,6 @@ private Object wrap(final Object nextval, final Object value) {
ParamReference<?> pref = (ParamReference) value;
return new StrParamReferenceImpl(pref.type(), pref.name(),
ImmutableList.of((String) nextval));
} else if (nextval instanceof Upload) {
ParamReference<?> pref = (ParamReference) value;
return new UploadParamReferenceImpl(pref.name(), ImmutableList.of((Upload) nextval));
}
return nextval;
}
Expand Down
6 changes: 6 additions & 0 deletions jooby/src/test/java/org/jooby/RequestTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.easymock.EasyMock.expect;
import static org.junit.Assert.assertEquals;

import java.io.IOException;
import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.HashMap;
Expand Down Expand Up @@ -234,6 +235,11 @@ public long timestamp() {
throw new UnsupportedOperationException();
}

@Override
public List<Upload> files(final String name) throws IOException {
throw new UnsupportedOperationException();
}

}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public class BuiltinParserTest {

@Test
public void values() {
assertEquals(6, BuiltinParser.values().length);
assertEquals(5, BuiltinParser.values().length);
}

@Test
Expand Down
Loading

0 comments on commit bdb4177

Please sign in to comment.