You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2020/02/06 18:32:04 UTC
[GitHub] [cxf] natechadwick opened a new pull request #637: Cxf 8208 -
Handle exceptions when looking up swagger ui resources
natechadwick opened a new pull request #637: Cxf 8208 - Handle exceptions when looking up swagger ui resources
URL: https://github.com/apache/cxf/pull/637
CXF-8208 add exception handler so that failed swagger ui resource lookups don't fail the request completely with invalid uri errors.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [cxf] natechadwick commented on a change in pull request #637: Cxf
8208 - Handle exceptions when looking up swagger ui resources
Posted by GitBox <gi...@apache.org>.
natechadwick commented on a change in pull request #637: Cxf 8208 - Handle exceptions when looking up swagger ui resources
URL: https://github.com/apache/cxf/pull/637#discussion_r376227845
##########
File path: rt/rs/description-swagger-ui/src/main/java/org/apache/cxf/jaxrs/swagger/ui/SwaggerUiResourceLocator.java
##########
@@ -50,8 +50,14 @@ public URL locate(String resourcePath) throws MalformedURLException {
if (resourcePath.startsWith("/")) {
resourcePath = resourcePath.substring(1);
}
+ URL ret;
- return URI.create(swaggerUiRoot + resourcePath).toURL();
+ try {
+ ret = URI.create(swaggerUiRoot + resourcePath).toURL();
+ } catch (Exception ex) {
Review comment:
I can do that, but the way that the code is currently implemented, the only caller is looking for a MalformedURLException if there is any problem in this method, and the request chain gets nuked if any other exception is thrown which is kinda catastrophic. Was thinking that any exception from this method should raise the MalformedURLException so the caller just returns false. Future proofing. If you still want the specific exception caught I can edit and resubmit.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [cxf] reta commented on issue #637: Cxf 8208 - Handle exceptions
when looking up swagger ui resources
Posted by GitBox <gi...@apache.org>.
reta commented on issue #637: Cxf 8208 - Handle exceptions when looking up swagger ui resources
URL: https://github.com/apache/cxf/pull/637#issuecomment-583937059
@natechadwick thank you, 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
With regards,
Apache Git Services
[GitHub] [cxf] reta commented on a change in pull request #637: Cxf 8208 -
Handle exceptions when looking up swagger ui resources
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #637: Cxf 8208 - Handle exceptions when looking up swagger ui resources
URL: https://github.com/apache/cxf/pull/637#discussion_r376656400
##########
File path: rt/rs/description-swagger-ui/src/main/java/org/apache/cxf/jaxrs/swagger/ui/SwaggerUiResourceLocator.java
##########
@@ -50,8 +50,14 @@ public URL locate(String resourcePath) throws MalformedURLException {
if (resourcePath.startsWith("/")) {
resourcePath = resourcePath.substring(1);
}
+ URL ret;
- return URI.create(swaggerUiRoot + resourcePath).toURL();
+ try {
+ ret = URI.create(swaggerUiRoot + resourcePath).toURL();
+ } catch (Exception ex) {
Review comment:
Thanks for the response, it is fine to raise `MalformedURLException` as you did, the `URI.create` does not do anything beside parsing and throwing `IllegalArgumentException`:
```
try {
return URI.create(swaggerUiRoot + resourcePath).toURL();
} catch (IllegalArgumentException ex) {
throw new MalformedURLException(ex.getMessage(), ex);
}
```
Thank you.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [cxf] amarkevich commented on a change in pull request #637: Cxf
8208 - Handle exceptions when looking up swagger ui resources
Posted by GitBox <gi...@apache.org>.
amarkevich commented on a change in pull request #637: Cxf 8208 - Handle exceptions when looking up swagger ui resources
URL: https://github.com/apache/cxf/pull/637#discussion_r376066476
##########
File path: rt/rs/description-swagger-ui/src/test/java/org/apache/cxf/jaxrs/swagger/ui/SwaggerUIResourceLocatorTest.java
##########
@@ -0,0 +1,46 @@
+/**
+ * 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.
+ */
+
+package org.apache.cxf.jaxrs.swagger.ui;
+
+import java.net.MalformedURLException;
+
+import org.junit.Test;
+
+
+
+public class SwaggerUIResourceLocatorTest {
+
+ @Test
Review comment:
use @Test(expected = MalformedURLException .class) instead of inner try-catch
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [cxf] coheigea merged pull request #637: Cxf 8208 - Handle
exceptions when looking up swagger ui resources
Posted by GitBox <gi...@apache.org>.
coheigea merged pull request #637: Cxf 8208 - Handle exceptions when looking up swagger ui resources
URL: https://github.com/apache/cxf/pull/637
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [cxf] amarkevich commented on a change in pull request #637: Cxf
8208 - Handle exceptions when looking up swagger ui resources
Posted by GitBox <gi...@apache.org>.
amarkevich commented on a change in pull request #637: Cxf 8208 - Handle exceptions when looking up swagger ui resources
URL: https://github.com/apache/cxf/pull/637#discussion_r376896483
##########
File path: rt/rs/description-swagger-ui/src/main/java/org/apache/cxf/jaxrs/swagger/ui/SwaggerUiResourceLocator.java
##########
@@ -50,8 +50,14 @@ public URL locate(String resourcePath) throws MalformedURLException {
if (resourcePath.startsWith("/")) {
resourcePath = resourcePath.substring(1);
}
+ URL ret;
- return URI.create(swaggerUiRoot + resourcePath).toURL();
+ try {
+ ret = URI.create(swaggerUiRoot + resourcePath).toURL();
+ } catch (Exception ex) {
Review comment:
To avoid additional knowledge about internal RuntimeException rising a regular constructor with checked URISyntaxException can be used:
```
try {
return new URI(swaggerUiRoot + resourcePath).toURL();
} catch (URISyntaxException ex) {
throw new MalformedURLException(ex.getMessage());
}
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [cxf] reta commented on a change in pull request #637: Cxf 8208 -
Handle exceptions when looking up swagger ui resources
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #637: Cxf 8208 - Handle exceptions when looking up swagger ui resources
URL: https://github.com/apache/cxf/pull/637#discussion_r376197865
##########
File path: rt/rs/description-swagger-ui/src/main/java/org/apache/cxf/jaxrs/swagger/ui/SwaggerUiResourceLocator.java
##########
@@ -50,8 +50,14 @@ public URL locate(String resourcePath) throws MalformedURLException {
if (resourcePath.startsWith("/")) {
resourcePath = resourcePath.substring(1);
}
+ URL ret;
- return URI.create(swaggerUiRoot + resourcePath).toURL();
+ try {
+ ret = URI.create(swaggerUiRoot + resourcePath).toURL();
+ } catch (Exception ex) {
Review comment:
The `URI.create` may thrown only `IllegalArgumentException` exception, could you please use it instead of generic `Exception`? Thank you.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [cxf] natechadwick commented on a change in pull request #637: Cxf
8208 - Handle exceptions when looking up swagger ui resources
Posted by GitBox <gi...@apache.org>.
natechadwick commented on a change in pull request #637: Cxf 8208 - Handle exceptions when looking up swagger ui resources
URL: https://github.com/apache/cxf/pull/637#discussion_r376229584
##########
File path: rt/rs/description-swagger-ui/src/test/java/org/apache/cxf/jaxrs/swagger/ui/SwaggerUIResourceLocatorTest.java
##########
@@ -0,0 +1,46 @@
+/**
+ * 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.
+ */
+
+package org.apache.cxf.jaxrs.swagger.ui;
+
+import java.net.MalformedURLException;
+
+import org.junit.Test;
+
+
+
+public class SwaggerUIResourceLocatorTest {
+
+ @Test
Review comment:
got it
----------------------------------------------------------------
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
With regards,
Apache Git Services