Skip to content

Commit

Permalink
fix: Unfriendly error for file uploading endpoint #508 (#559)
Browse files Browse the repository at this point in the history
  • Loading branch information
astsiapanay authored Nov 4, 2024
1 parent 8a500af commit 83c1077
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@
import com.epam.aidial.core.server.util.ResourceDescriptorFactory;
import com.epam.aidial.core.server.vertx.stream.BlobWriteStream;
import com.epam.aidial.core.storage.data.FileMetadata;
import com.epam.aidial.core.storage.http.HttpException;
import com.epam.aidial.core.storage.http.HttpStatus;
import com.epam.aidial.core.storage.resource.ResourceDescriptor;
import com.epam.aidial.core.storage.util.EtagHeader;
import io.netty.handler.codec.http.HttpHeaderNames;
import io.vertx.core.Future;
import io.vertx.core.buffer.Buffer;
import io.vertx.core.http.HttpHeaders;
import io.vertx.core.http.HttpServerRequest;
import io.vertx.core.http.impl.HttpUtils;
import io.vertx.core.streams.Pipe;
import io.vertx.core.streams.impl.PipeImpl;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -34,8 +38,7 @@ protected Future<?> handle(ResourceDescriptor resource, boolean hasWriteAccess)
}

return proxy.getVertx().executeBlocking(() -> {
EtagHeader etag = ProxyUtil.etag(context.getRequest());
etag.validate(() -> proxy.getResourceService().getEtag(resource));
EtagHeader etag = validateRequest(context.getRequest(), resource);
context.getRequest()
.setExpectMultipart(true)
.uploadHandler(upload -> {
Expand Down Expand Up @@ -65,4 +68,17 @@ protected Future<?> handle(ResourceDescriptor resource, boolean hasWriteAccess)
return null;
});
}

private EtagHeader validateRequest(HttpServerRequest request, ResourceDescriptor resource) {
EtagHeader etag = ProxyUtil.etag(context.getRequest());
String contentType = request.headers().get(HttpHeaderNames.CONTENT_TYPE);
etag.validate(() -> proxy.getResourceService().getEtag(resource));
if (contentType == null) {
throw new HttpException(HttpStatus.BAD_REQUEST, "Request must have a content-type header to decode a multipart request");
}
if (!HttpUtils.isValidMultipartContentType(contentType)) {
throw new HttpException(HttpStatus.BAD_REQUEST, "Request must have a valid content-type header to decode a multipart request");
}
return etag;
}
}
54 changes: 54 additions & 0 deletions server/src/test/java/com/epam/aidial/core/server/FileApiTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,60 @@ public void testFileUploadWithInvalidPath(Vertx vertx, VertxTestContext context)
});
}

@Test
public void testUploadFileWithWrongContentType(Vertx vertx, VertxTestContext context) {
Checkpoint checkpoint = context.checkpoint(2);
WebClient client = WebClient.create(vertx);

Future.succeededFuture().compose((mapper) -> {
Promise<Void> promise = Promise.promise();
// verify no files
client.get(serverPort, "localhost", "/v1/metadata/files/7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/?permissions=true")
.putHeader("Api-key", "proxyKey2")
.as(BodyCodec.string())
.send(context.succeeding(response -> {
context.verify(() -> {
assertEquals(200, response.statusCode());
verifyJsonNotExact("""
{
"name" : null,
"parentPath" : null,
"bucket" : "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt",
"url" : "files/7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/",
"nodeType" : "FOLDER",
"resourceType" : "FILE",
"permissions" : [ "READ", "WRITE" ],
"items" : [ ]
}
""", response.body());
checkpoint.flag();
promise.complete();
});
}));

return promise.future();
}).compose((mapper) -> {
Promise<Void> promise = Promise.promise();
// upload test file with wrong content type
client.put(serverPort, "localhost", "/v1/files/7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/%D1%84%D0%B0%D0%B9%D0%BB.txt")
.putHeader("Api-key", "proxyKey2")
.putHeader("content-type", "wrong-multipart/form-data")
.as(BodyCodec.string())
.send(
context.succeeding(response -> {
context.verify(() -> {
assertEquals(400, response.statusCode());
assertEquals("Request must have a valid content-type header to decode a multipart request", response.body());
checkpoint.flag();
promise.complete();
});
})
);

return promise.future();
});
}

@Test
public void testFileUpload(Vertx vertx, VertxTestContext context) {
Checkpoint checkpoint = context.checkpoint(4);
Expand Down

0 comments on commit 83c1077

Please sign in to comment.