Skip to content

Commit d482924

Browse files
lachlan-robertslachlan-roberts
committed
updates to use jetty 12.1.1
Signed-off-by: Lachlan Roberts <[email protected]>
1 parent b1566b1 commit d482924

File tree

9 files changed

+166
-165
lines changed

9 files changed

+166
-165
lines changed

‎invoker/core/src/main/java/com/google/cloud/functions/invoker/HttpFunctionExecutor.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,19 +73,13 @@ public boolean handle(Request request, Response response, Callback callback) thr
7373
HttpResponseImpl respImpl = new HttpResponseImpl(response);
7474
ClassLoader oldContextLoader = Thread.currentThread().getContextClassLoader();
7575
try {
76-
executionIdUtil.storeExecutionId(req);
76+
executionIdUtil.storeExecutionId(request);
7777
Thread.currentThread().setContextClassLoader(function.getClass().getClassLoader());
7878
function.service(reqImpl, respImpl);
7979
respImpl.close(callback);
8080
} catch (Throwable t) {
8181
logger.log(Level.SEVERE, "Failed to execute " + function.getClass().getName(), t);
82-
if (response.isCommitted()) {
83-
callback.failed(t);
84-
} else {
85-
response.reset();
86-
response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500);
87-
callback.succeeded();
88-
}
82+
Response.writeError(request, response, callback, HttpStatus.INTERNAL_SERVER_ERROR_500, null, t);
8983
} finally {
9084
Thread.currentThread().setContextClassLoader(oldContextLoader);
9185
executionIdUtil.removeExecutionId();

‎invoker/core/src/main/java/com/google/cloud/functions/invoker/TypedFunctionExecutor.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,7 @@ public boolean handle(Request req, Response res, Callback callback) throws Excep
106106
handleRequest(reqImpl, resImpl);
107107
resImpl.close(callback);
108108
} catch (Throwable t) {
109-
if (res.isCommitted()) {
110-
callback.failed(t);
111-
} else {
112-
res.reset();
113-
res.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500);
114-
callback.succeeded();
115-
}
109+
Response.writeError(req, res, callback, HttpStatus.INTERNAL_SERVER_ERROR_500, null, t);
116110
} finally {
117111
Thread.currentThread().setContextClassLoader(oldContextClassLoader);
118112
}

‎invoker/core/src/main/java/com/google/cloud/functions/invoker/gcf/ExecutionIdUtil.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import java.util.concurrent.ThreadLocalRandom;
66
import java.util.logging.Handler;
77
import java.util.logging.Logger;
8-
import javax.servlet.http.HttpServletRequest;
8+
import org.eclipse.jetty.server.Request;
99

1010
/**
1111
* A helper class that either fetches a unique execution id from request HTTP headers or generates a
@@ -23,7 +23,7 @@ public final class ExecutionIdUtil {
2323
* Add mapping to root logger from current thread id to execution id. This mapping will be used to
2424
* append the execution id to log lines.
2525
*/
26-
public void storeExecutionId(HttpServletRequest request) {
26+
public void storeExecutionId(Request request) {
2727
if (!executionIdLoggingEnabled()) {
2828
return;
2929
}
@@ -47,8 +47,8 @@ public void removeExecutionId() {
4747
}
4848
}
4949

50-
private String getOrGenerateExecutionId(HttpServletRequest request) {
51-
String executionId = request.getHeader(EXECUTION_ID_HTTP_HEADER);
50+
private String getOrGenerateExecutionId(Request request) {
51+
String executionId = request.getHeaders().get(EXECUTION_ID_HTTP_HEADER);
5252
if (executionId == null) {
5353
byte[] array = new byte[EXECUTION_ID_LENGTH];
5454
random.nextBytes(array);

‎invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpRequestImpl.java‎

Lines changed: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,8 @@
2929
import java.util.Objects;
3030
import java.util.Optional;
3131
import java.util.TreeMap;
32-
import java.util.concurrent.ExecutionException;
33-
import org.eclipse.jetty.http.HttpField;
34-
import org.eclipse.jetty.http.HttpFields;
35-
import org.eclipse.jetty.http.HttpHeader;
36-
import org.eclipse.jetty.http.MimeTypes;
37-
import org.eclipse.jetty.http.MultiPart;
32+
import org.eclipse.jetty.http.*;
3833
import org.eclipse.jetty.http.MultiPart.Part;
39-
import org.eclipse.jetty.http.MultiPartFormData;
4034
import org.eclipse.jetty.io.Content;
4135
import org.eclipse.jetty.server.Request;
4236
import org.eclipse.jetty.util.Fields;
@@ -78,39 +72,31 @@ public Map<String, List<String>> getQueryParameters() {
7872
}
7973

8074
Map<String, List<String>> map = new HashMap<>();
81-
fields.forEach(field -> map.put(field.getName(),
82-
Collections.unmodifiableList(field.getValues())));
75+
fields.forEach(
76+
field -> map.put(field.getName(), Collections.unmodifiableList(field.getValues())));
8377
return Collections.unmodifiableMap(map);
8478
}
8579

8680
@Override
8781
public Map<String, HttpPart> getParts() {
88-
// TODO initiate reading the parts asynchronously before invocation
8982
String contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE);
90-
if (contentType == null || !contentType.startsWith("multipart/form-data")) {
83+
if (contentType == null || !contentType.startsWith(MimeTypes.Type.MULTIPART_FORM_DATA.asString())) {
9184
throw new IllegalStateException("Content-Type must be multipart/form-data: " + contentType);
9285
}
93-
String boundary = MultiPart.extractBoundary(contentType);
94-
if (boundary == null) {
95-
throw new IllegalStateException("No boundary in content-type: " + contentType);
96-
}
97-
try {
98-
MultiPartFormData.Parts parts =
99-
MultiPartFormData.from(request, boundary, parser -> {
100-
parser.setMaxMemoryFileSize(-1);
101-
return parser.parse(request);
102-
}).get();
10386

104-
if (parts.size() == 0) {
105-
return Collections.emptyMap();
106-
}
87+
// The multipart parsing is done by the EagerContentHandler, so we just call getParts.
88+
MultiPartFormData.Parts parts = MultiPartFormData.getParts(request);
89+
if (parts == null){
90+
throw new IllegalStateException();
91+
}
10792

108-
Map<String, HttpPart> map = new HashMap<>();
109-
parts.forEach(part -> map.put(part.getName(), new HttpPartImpl(part)));
110-
return Collections.unmodifiableMap(map);
111-
} catch (InterruptedException | ExecutionException e) {
112-
throw new RuntimeException(e);
93+
if (parts.size() == 0) {
94+
return Collections.emptyMap();
11395
}
96+
97+
Map<String, HttpPart> map = new HashMap<>();
98+
parts.forEach(part -> map.put(part.getName(), new HttpPartImpl(part)));
99+
return Collections.unmodifiableMap(map);
114100
}
115101

116102
@Override
@@ -147,8 +133,11 @@ public BufferedReader getReader() throws IOException {
147133
throw new IllegalStateException("getInputStream already called");
148134
}
149135
inputStream = Content.Source.asInputStream(request);
150-
reader = new BufferedReader(new InputStreamReader(getInputStream(),
151-
Objects.requireNonNullElse(Request.getCharset(request), StandardCharsets.UTF_8)));
136+
reader =
137+
new BufferedReader(
138+
new InputStreamReader(
139+
getInputStream(),
140+
Objects.requireNonNullElse(Request.getCharset(request), StandardCharsets.UTF_8)));
152141
}
153142
return reader;
154143
}
@@ -175,10 +164,6 @@ private HttpPartImpl(Part part) {
175164
contentType = part.getHeaders().get(HttpHeader.CONTENT_TYPE);
176165
}
177166

178-
public String getName() {
179-
return part.getName();
180-
}
181-
182167
@Override
183168
public Optional<String> getFileName() {
184169
return Optional.ofNullable(part.getFileName());
@@ -201,15 +186,18 @@ public Optional<String> getCharacterEncoding() {
201186

202187
@Override
203188
public InputStream getInputStream() throws IOException {
204-
return Content.Source.asInputStream(part.newContentSource());
189+
// TODO: update with createContentSource when https://github.com/jetty/jetty.project/pull/13610 is released.
190+
Content.Source contentSource = part.newContentSource(null, 0, -1);
191+
return Content.Source.asInputStream(contentSource);
205192
}
206193

207194
@Override
208195
public BufferedReader getReader() throws IOException {
209196
return new BufferedReader(
210-
new InputStreamReader(getInputStream(),
211-
Objects.requireNonNullElse(MimeTypes.DEFAULTS.getCharset(contentType),
212-
StandardCharsets.UTF_8)));
197+
new InputStreamReader(
198+
getInputStream(),
199+
Objects.requireNonNullElse(
200+
MimeTypes.DEFAULTS.getCharset(contentType), StandardCharsets.UTF_8)));
213201
}
214202

215203
@Override

‎invoker/core/src/main/java/com/google/cloud/functions/invoker/http/TimeoutFilter.java‎

Lines changed: 0 additions & 71 deletions
This file was deleted.

‎invoker/core/src/main/java/com/google/cloud/functions/invoker/http/TimeoutHandler.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright 2024 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.cloud.functions.invoker.http;
16+
17+
import java.util.Timer;
18+
import java.util.TimerTask;
19+
import java.util.concurrent.atomic.AtomicBoolean;
20+
import org.eclipse.jetty.http.HttpStatus;
21+
import org.eclipse.jetty.server.Handler;
22+
import org.eclipse.jetty.server.Request;
23+
import org.eclipse.jetty.server.Response;
24+
import org.eclipse.jetty.util.Callback;
25+
26+
public class TimeoutHandler extends Handler.Wrapper {
27+
private final int timeoutMs;
28+
29+
public TimeoutHandler(int timeoutSeconds, Handler handler) {
30+
setHandler(handler);
31+
this.timeoutMs = timeoutSeconds * 1000; // Convert seconds to milliseconds
32+
}
33+
34+
@Override
35+
public boolean handle(Request request, Response response, Callback callback) throws Exception {
36+
// Wrap the callback to ensure it is only called once between the handler and the timeout task.
37+
AtomicBoolean completed = new AtomicBoolean(false);
38+
Callback wrappedCallback = new Callback() {
39+
@Override
40+
public void succeeded() {
41+
if (completed.compareAndSet(false, true)) {
42+
callback.succeeded();
43+
}
44+
}
45+
46+
@Override
47+
public void failed(Throwable x) {
48+
if (completed.compareAndSet(false, true)) {
49+
callback.failed(x);
50+
}
51+
}
52+
53+
@Override
54+
public InvocationType getInvocationType() {
55+
return callback.getInvocationType();
56+
}
57+
};
58+
59+
// TODO: consider wrapping the request/response to throw if they are used after timeout.
60+
// TODO: Use org.eclipse.jetty.io.CyclicTimeouts which is optimized for timeouts which are almost always cancelled.
61+
Timer timer = new Timer(true);
62+
TimerTask timeoutTask =
63+
new TimerTask() {
64+
@Override
65+
public void run() {
66+
// TODO: there is a race between the handler writing response and timeout firing.
67+
// This timeout firing doesn't stop the thread handling the request / response it just writes an error to the response.
68+
Response.writeError(
69+
request,
70+
response,
71+
callback,
72+
HttpStatus.REQUEST_TIMEOUT_408,
73+
"Function execution timed out");
74+
}
75+
};
76+
77+
timer.schedule(timeoutTask, timeoutMs);
78+
79+
boolean handle;
80+
try {
81+
handle = super.handle(request, response, wrappedCallback);
82+
timeoutTask.cancel();
83+
} finally {
84+
timer.purge();
85+
}
86+
87+
return handle;
88+
}
89+
}

0 commit comments

Comments
 (0)