You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2021/04/08 20:42:20 UTC
[GitHub] [httpcomponents-core] ok2c opened a new pull request #282: Improved parsing and formatting of URI components
ok2c opened a new pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282
@carterkozak Please review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611228377
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
Almost perfect, just pass the `Host` instance directly instead of having another instance created by the constructor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#issuecomment-817354739
Looking at `HttpHost` again, it can wipe internal fields as well for a `Host` instance just like you have done with `URIAuthority`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611222270
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
I'd rather prefer to pass the `Host` instance directly and get rid of the actual private fields.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#issuecomment-817357731
>
>
> > Looking at `HttpHost` again, it can wipe internal fields as well for a `Host` instance just like you have done with `URIAuthority`.
>
> @michael-o What would be the benefit of doing so?
Code reduction and reuse. Plus a clear logical hierarchy (not talking about inheritance).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] carterkozak commented on pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#issuecomment-817308543
I believe the plan was to merge this before my change -- after a rebase it should make my modifications much simpler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611218688
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
Not extend, but `URIAuthority(UserInfo, Host)` instead of `URIAuthority(UserInfo, String, int)`. Thus, not maintain the raw fields internally, but use `Host`. Delegation, not extension.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611227791
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
@michael-o Please take another look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c merged pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
ok2c merged pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#issuecomment-817288335
I will review this next week. I already see issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611218097
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
@michael-o I am not sure I understand what you mean. Why does not `URIAuthority` extend `Host`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611221696
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
@michael-o Please take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611228991
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
> Almost perfect, just pass the `Host` instance directly instead of having another instance created by the constructor.
@michael-o I already do, don't I? See line 107
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] arturobernalg commented on pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#issuecomment-817302435
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611210813
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
Stupid question, why can't a `URIAuthority` be backed by a `Host` just like `HttpHost` does?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611231261
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
@michael-o Corrected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#issuecomment-817290185
@ok2c Should this be merged before Carter's work?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611219179
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
@michael-o No reason, actually. This can be easily done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#issuecomment-817356428
> Looking at `HttpHost` again, it can wipe internal fields as well for a `Host` instance just like you have done with `URIAuthority`.
@michael-o What would be the benefit of doing so?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611231969
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
Very nice now!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611229882
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java
##########
@@ -87,23 +64,8 @@ static URIAuthority parse(final CharSequence s, final Tokenizer.Cursor cursor) t
//Rewind
cursor.updatePos(initPos);
}
- final String hostName = tokenizer.parseContent(s, cursor, PORT_SEPARATORS);
- String portText = null;
- if (!cursor.atEnd() && s.charAt(cursor.getPos()) == ':') {
- cursor.updatePos(cursor.getPos() + 1);
- portText = tokenizer.parseContent(s, cursor, TERMINATORS);
- }
- final int port;
- if (!TextUtils.isBlank(portText)) {
- try {
- port = Integer.parseInt(portText);
- } catch (final NumberFormatException ex) {
- throw createException(s, cursor, "Authority port is invalid");
- }
- } else {
- port = -1;
- }
- return new URIAuthority(userInfo, hostName.toLowerCase(Locale.ROOT), port, true);
+ final Host host = Host.parse(s, cursor);
+ return new URIAuthority(userInfo, host.getHostName(), host.getPort());
Review comment:
Yes, you do, but here
https://github.com/apache/httpcomponents-core/blob/d8296ab690f7d45b6d659878e86a5c870f2d00db/httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java#L66-L67
you use the getters and create new object here again
https://github.com/apache/httpcomponents-core/blob/d8296ab690f7d45b6d659878e86a5c870f2d00db/httpcore5/src/main/java/org/apache/hc/core5/net/URIAuthority.java#L94-L98
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#issuecomment-817492314
> Code reduction and reuse. Plus a clear logical hierarchy (not talking about inheritance).
@michael-o I am not entirely sure if it is really worth it but so be it. Please review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] carterkozak commented on a change in pull request #282: Improved parsing and formatting of URI components
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #282:
URL: https://github.com/apache/httpcomponents-core/pull/282#discussion_r611546177
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URISupport.java
##########
@@ -0,0 +1,58 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation. For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.core5.net;
+
+import java.net.URISyntaxException;
+import java.util.BitSet;
+
+import org.apache.hc.core5.util.Tokenizer;
+
+final class URISupport {
+
+ static final BitSet HOST_SEPARATORS = new BitSet(256);
+ static final BitSet PORT_SEPARATORS = new BitSet(256);
+ static final BitSet TERMINATORS = new BitSet(256);
+
+ static {
+ TERMINATORS.set('/');
+ TERMINATORS.set('#');
+ TERMINATORS.set('?');
+ HOST_SEPARATORS.or(TERMINATORS);
+ HOST_SEPARATORS.set('@');
+ PORT_SEPARATORS.or(TERMINATORS);
+ PORT_SEPARATORS.set(':');
+ }
+
+ static URISyntaxException createException(
+ final CharSequence input, final Tokenizer.Cursor cursor, final String reason) {
+ return new URISyntaxException(
+ input.subSequence(cursor.getLowerBound(), cursor.getUpperBound()).toString(),
+ reason,
+ cursor.getPos());
+ }
+
Review comment:
I'd add a private ctor so it's clear this is a utility class for static functionality, but it's private API so it doesn't make much difference
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org