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 23, 2017
1 parent 050b82f commit 7737225
Show file tree
Hide file tree
Showing 20 changed files with 124 additions and 192 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 @@ -2669,7 +2669,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 @@ -264,8 +264,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 @@ -283,17 +282,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 @@ -281,25 +281,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 @@ -20,6 +20,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 @@ -232,12 +233,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 @@ -828,9 +829,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 @@ -839,10 +858,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 @@ -35,7 +35,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 @@ -138,13 +137,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 @@ -174,8 +166,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 @@ -206,17 +196,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 @@ -21,6 +21,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 @@ -207,27 +208,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

This file was deleted.

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 @@ -21,6 +21,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 @@ -33,6 +34,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 @@ -87,6 +89,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 @@ -122,7 +137,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 @@ -24,11 +24,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 @@ -95,17 +93,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 @@ -32,10 +32,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 @@ -100,16 +98,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 @@ -171,9 +159,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
Loading

0 comments on commit 7737225

Please sign in to comment.