smallrye-graphql icon indicating copy to clipboard operation
smallrye-graphql copied to clipboard

Validation constraints exception with regex on printing schema

Open robp94 opened this issue 2 years ago • 3 comments

When I activate quarkus.smallrye-graphql.schema-include-directives=true and use a regex like this

@Pattern(regexp = "^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]")

I get an error like this when I try to print the schema

java.util.UnknownFormatConversionException: Conversion = '&'
	at java.base/java.util.Formatter.parse(Formatter.java:2750)
	at java.base/java.util.Formatter.format(Formatter.java:2671)
	at java.base/java.io.PrintWriter.format(PrintWriter.java:990)
	at graphql.schema.idl.SchemaPrinter.lambda$null$9(SchemaPrinter.java:678)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.stream.SortedOps$RefSortingSink.end(SortedOps.java:395)
	at java.base/java.util.stream.Sink$ChainedReference.end(Sink.java:258)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:510)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at graphql.schema.idl.SchemaPrinter.lambda$inputObjectPrinter$10(SchemaPrinter.java:669)
	at graphql.schema.idl.SchemaPrinter.printSchemaElement(SchemaPrinter.java:1013)
	at graphql.schema.idl.SchemaPrinter.print(SchemaPrinter.java:446)
	at io.smallrye.graphql.execution.SchemaPrinter.print(SchemaPrinter.java:15)
	at io.quarkus.smallrye.graphql.runtime.SmallRyeGraphQLSchemaHandler.handle(SmallRyeGraphQLSchemaHandler.java:35)
	at io.quarkus.smallrye.graphql.runtime.SmallRyeGraphQLSchemaHandler.handle(SmallRyeGraphQLSchemaHandler.java:18)
	at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1284)
	at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:173)
	at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:140)
	at io.quarkus.vertx.http.runtime.VertxHttpRecorder$6.handle(VertxHttpRecorder.java:422)
	at io.quarkus.vertx.http.runtime.VertxHttpRecorder$6.handle(VertxHttpRecorder.java:400)
	at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1284)
	at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:173)
	at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:140)
	at io.quarkus.vertx.http.runtime.devmode.VertxHttpHotReplacementSetup$5.handle(VertxHttpHotReplacementSetup.java:196)
	at io.quarkus.vertx.http.runtime.devmode.VertxHttpHotReplacementSetup$5.handle(VertxHttpHotReplacementSetup.java:185)
	at io.vertx.core.impl.future.FutureImpl$3.onSuccess(FutureImpl.java:141)
	at io.vertx.core.impl.future.FutureBase.lambda$emitSuccess$0(FutureBase.java:54)
	at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:174)
	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:167)
	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:503)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:833)

I looked into it, it seems that the used formatter does not support "%"

jdk 11 java.util.Formatter#checkText
            if (s.charAt(i) == '%') {
               char c = (i == end - 1) ? '%' : s.charAt(i + 1);
               throw new UnknownFormatConversionException(String.valueOf(c));
           }

We would need to escape percentages with another percentage sign. With a description which includes a percentage sign, there is no problem. So there is not a general problem with percentages. Or is it handled differently? code-with-quarkus-percentage.zip

robp94 avatar Aug 08 '22 11:08 robp94

This seems to be related to the way how graphql-java prints directives - this call: https://github.com/graphql-java/graphql-java/blob/v18.1/src/main/java/graphql/schema/idl/SchemaPrinter.java#L678 is the culprit, I think it should have been a plain print instead of format, because it does not specify any formatting arguments, so if the string itself contains a formatting specifier, it will blow up. I tend to think this is an issue in graphql-java ?! We could potentially add escaping to things that look like formatting specifiers, that might be a workaround, but I haven't checked yet... Can try later.

jmartisk avatar Aug 08 '22 12:08 jmartisk

I've submitted a fix straight to graphql-java: https://github.com/graphql-java/graphql-java/pull/2920 Let's see if it gets accepted. I've also prepared a test that we will add on our side once we upgrade to a graphql-java version containing the fix. (The test is in this branch for now https://github.com/jmartisk/smallrye-graphql/tree/1.7.x-test-for-1492)

jmartisk avatar Aug 15 '22 07:08 jmartisk

Fix merged into graphql-java, so this will be resolved on our side with an upgrade to graphql-java 20, when it's released

jmartisk avatar Aug 15 '22 11:08 jmartisk

SmallRye GraphQL 2.2 is on graphql-java 20, so I think this can be closed

jmartisk avatar Jun 13 '23 09:06 jmartisk