Comments (6)
Hi, @ikhoon, could I try this issue?
from armeria.
Hi sorry, we're doing an internal open source sprint and this issue has been assigned 😅 Let us assign labels on such issues so that it is clear which issues are already being worked on 🙇
from armeria.
Hi sorry, we're doing an internal open source sprint and this issue has been assigned 😅 Let us assign labels on such issues so that it is clear which issues are already being worked on 🙇
sure, so what kind of label could I take on?
from armeria.
Armeria provides a HTTP client WebClient, which is able to send HTTP requests.
Usually, a user provides a String format URI and WebClient
needs to parse the different parts of the URI to determine the correct protocol, endpoint, and various headers.
URI = scheme ":" hier-part [ "?" query ] [ "#" fragment ]
hier-part = "//" authority path-abempty
/ path-absolute
/ path-rootless
/ path-empty
ref: https://www.rfc-editor.org/rfc/rfc3986.html#section-3
Amongst the different URI components, the authority determines 1) Where to send the request 2) What to set to the Host
or :authority
header. Within Armeria, we 1) strip the userinfo
2) separate the host and port 3) and validate the host and port values.
authority = [ userinfo "@" ] host [ ":" port ]
ref: https://www.rfc-editor.org/rfc/rfc3986.html#section-3.2
However, the validation/parsing logic of the authority is fragmented across the code base:
armeria/core/src/main/java/com/linecorp/armeria/client/Endpoint.java
Lines 104 to 115 in db3973d
The objective of this issue is to unify the validation/parsing logic above
To get jump-started with the current code base, it's probably best one checks from unit tests how the above logic works.
Here are some code snippets which should get one started with how the Armeria public APIs interact with the above logic.
i.e. When sending the request from WebClient
class WebClientTest {
@RegisterExtension
static final ServerExtension server = new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) throws Exception {
sb.service("/", (ctx, req) -> HttpResponse.of(200));
}
};
@Test
void test1() {
final AggregatedHttpResponse res = WebClient.of().blocking().get(server.httpUri().toString());
assertThat(res.status().code()).isEqualTo(200);
}
@Test
void test2() {
final HttpRequest req = HttpRequest.builder()
.header(HttpHeaderNames.SCHEME, "http")
.header(HttpHeaderNames.AUTHORITY, server.httpUri().getRawAuthority())
.get("/")
.build();
final AggregatedHttpResponse res = WebClient.of().blocking().execute(req);
assertThat(res.status().code()).isEqualTo(200);
}
@Test
void test3() {
final AggregatedHttpResponse res = WebClient.of(server.httpUri()).blocking().get("/");
assertThat(res.status().code()).isEqualTo(200);
}
@Test
void test4() {
final Endpoint endpoint = Endpoint.of(server.httpUri().getRawAuthority());
final AggregatedHttpResponse res = WebClient.of(SessionProtocol.HTTP, endpoint).blocking().get("/");
assertThat(res.status().code()).isEqualTo(200);
}
}
i.e. When altering headers from decorators
@Test
void test5() {
final AggregatedHttpResponse res = server
.blockingWebClient(cb -> cb.decorator((delegate, ctx, req) -> {
ctx.addAdditionalRequestHeader(HttpHeaderNames.AUTHORITY, "my-authority:8080");
assertThat(ctx.host()).isEqualTo("my-authority");
return delegate.execute(ctx, req);
}))
.get("/");
assertThat(res.status().code()).isEqualTo(200);
}
i.e. When parsing an arbitrary target string (this method is used in RedirectingClient
/gRPC
clients)
@Test
void test6() {
final RequestTarget requestTarget = RequestTarget.forClient("http://my-authority:8080/hello");
assertThat(requestTarget.authority()).isEqualTo("my-authority:8080");
}
Afterwards, I recommend that we add a public method in ArmeriaHttpUtil and try to unify the above logic. The skeletal API would probably look something like the following:
public static ... parseAuthority(...) {
...
}
from armeria.
sure, so what kind of label could I take on?
Let me add a sprint
label to all affected issues
from armeria.
Also note the following bullet points from the initial report:
- Always strip the user info part from authority before validation and
- Always use new URI(String) to parse an authority string; and
- Make sure we move this logic into ArmeriaHttpUtil so it is shared by DefaultRequestTarget, Endpoint and DefaultClientRequestContext.
For starters, we may want to make this the initial implementation and trying to run the build. We could work on fixing each test case as we move forward.
from armeria.
Related Issues (20)
- Some dependencies in /core module are in the wrong scope. HOT 5
- Provide a way to ignore dependencies when performing `dependencyUpdates`
- Add /info URL to provide information about server application HOT 2
- Decouple HTTP/2 pings from idle timeout machinery and make then useful on their own
- Write docs for `context-propagation`
- Support for Parameter Options in Parameterized Path
- Make sure all tests pass when io_uring is enabled HOT 1
- Documentation for OAuth 2.0 client
- RequestTimeout is 503 with ThrottlingStrategy HOT 3
- Make `defaultUseHttp2Preface` flag more explicit
- Mask request and response logs from HTTP client HOT 1
- Automatically remove multipart temporary files
- Fix `socketConnect`, `pendingAcquisition` timing end method check condition
- Provide a way to customize meters
- Add pendingResponses to ServerMetrics
- Add ServerBuilder.coroutinService as Kotlin extension method
- Test failure: `com.linecorp.armeria.common.ContextPushHookTest.shouldRunHooksWhenContextIsPushed()`
- Test failure: `com.linecorp.armeria.server.InvalidPathWithDataTest.invalidPath()`
- Investigate if we can stop `ServiceRequestContextBuilder.build()` from scheduling an event loop task
- Use `SELF` for Builder Self Type Parameter
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from armeria.