Copilot commented on code in PR #15558:
URL: https://github.com/apache/grails-core/pull/15558#discussion_r3789665709
##########
grails-views-gson/src/main/groovy/grails/plugin/json/view/api/GrailsJsonViewHelper.groovy:
##########
@@ -45,7 +45,18 @@ interface GrailsJsonViewHelper extends GrailsViewHelper {
* @param arguments The named arguments: 'template', 'collection',
'model', 'var' and 'bean'
* @return The unescaped JSON
*/
- JsonOutput.JsonWritable render(Map arguments)
+ // Groovy 6 Verifier workaround (blocker #6): declared as `default`
(concrete) rather than
+ // abstract. Under Groovy 6.0.0-SNAPSHOT the concrete render(...)
overrides in
+ // DefaultGrailsJsonViewHelper get a different return-type descriptor than
these interface
+ // methods (inner-class return type JsonOutput.JsonWritable;
groovy.json.JsonOutput.JsonWritable
+ // was removed in Groovy 6), so the abstract-method check in
+ // ClassCompletionVerifier.checkNoAbstractMethodsNonAbstractClass
spuriously reports them
+ // unimplemented. Making them default removes them from
getAbstractMethods() so the check has
+ // nothing to flag; every real implementor overrides them. Remove once the
upstream regression
+ // is fixed.
+ default JsonOutput.JsonWritable render(Map arguments) {
+ throw new UnsupportedOperationException()
Review Comment:
These temporary default methods throw UnsupportedOperationException with no
message, which makes failures harder to diagnose if a custom implementation
accidentally hits the fallback. Include a message (and apply the same change to
the other temporary default render(...) overloads).
##########
grails-testing-support-http-client/src/main/groovy/org/apache/grails/testing/http/client/utils/XmlUtils.groovy:
##########
@@ -45,28 +45,42 @@ import org.xml.sax.SAXException
@CompileStatic
class XmlUtils {
- private static final String DISALLOW_DOCTYPE_DECL =
'https://apache.org/xml/features/disallow-doctype-decl'
- private static final String EXTERNAL_GENERAL_ENTITIES =
'https://xml.org/sax/features/external-general-entities'
- private static final String EXTERNAL_PARAMETER_ENTITIES =
'https://xml.org/sax/features/external-parameter-entities'
+ // SAX/Xerces feature identifiers are namespace-style URIs that use the
http scheme; the parser
+ // matches them by exact string, so https variants throw
SAXNotRecognizedException, get swallowed
+ // below, and silently leave the parser at its (JDK-version-dependent)
defaults.
+ private static final String DISALLOW_DOCTYPE_DECL =
'http://apache.org/xml/features/disallow-doctype-decl'
+ private static final String EXTERNAL_PARAMETER_ENTITIES =
'http://xml.org/sax/features/external-parameter-entities'
private static final String FEATURE_SECURE_PROCESSING =
XMLConstants.FEATURE_SECURE_PROCESSING
- private static final String LOAD_DTD_GRAMMAR =
'https://apache.org/xml/features/nonvalidating/load-dtd-grammar'
- private static final String LOAD_EXTERNAL_DTD =
'https://apache.org/xml/features/nonvalidating/load-external-dtd'
+ private static final String LOAD_DTD_GRAMMAR =
'http://apache.org/xml/features/nonvalidating/load-dtd-grammar'
+ private static final String LOAD_EXTERNAL_DTD =
'http://apache.org/xml/features/nonvalidating/load-external-dtd'
private static final Pattern SPACE_AND_EMPTY_ELEMENT_CLOSE = ~/ \/>/
private static final String EMPTY_ELEMENT_CLOSE = '/>'
private static final Pattern LINE_ENDINGS = ~/\r\n|[\r\n]/
private static final Pattern XML_DECLARATION = ~/^\s*(<\?xml\b.*?\?>)/
+ // Inline DOCTYPE with internal entities is allowed
(disallow-doctype-decl=false). External general
+ // entities are intentionally left enabled so a SYSTEM reference is
*attempted* and then blocked by
+ // the accessExternalDTD/Schema properties below, which throws a
SAXParseException ("External Entity:
+ // ... access is not allowed") rather than silently dropping the reference.
private static final Map<String, Boolean> SECURE_XML_SLURPER_FEATURES = [
Review Comment:
SECURE_XML_SLURPER_FEATURES no longer disables external general entities.
Because setProperty() calls are swallowed when unsupported, relying only on
ACCESS_EXTERNAL_* can leave external entity expansion enabled on some parsers,
reopening XXE/SSRF risk. Disable external-general-entities as defense-in-depth.
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/ConfigurationBuilder.groovy:
##########
@@ -463,4 +452,100 @@ abstract class ConfigurationBuilder<B, C> {
protected void startBuild(Object builder, String configurationPath) {
// no-op
}
+ /**
+ * Handle ConversionFailedException - for enums, try case-insensitive
conversion
+ */
+ private Object handleConversionException(ConversionFailedException e,
Class argType, String propertyPathForArg) {
+ if (argType.isEnum()) {
+ def value = propertyResolver.getProperty(propertyPathForArg,
String)
+ if (value != null) {
+ try {
+ return Enum.valueOf((Class) argType, value.toUpperCase())
+ } catch (Throwable e2) {
+ // ignore e2 and throw original
+ throw new ConfigurationException("Invalid value for
setting [$propertyPathForArg]: $e.message", e)
Review Comment:
GString interpolation here uses "$e.message", which renders as
"<exceptionToString>.message" rather than the exception message. Use
"${e.message}" so the ConfigurationException text is correct.
This issue also appears in the following locations of the same file:
- line 533
- line 546
##########
grails-test-suite-uber/src/test/groovy/org/grails/plugins/CoreGrailsPluginTests.groovy:
##########
@@ -19,22 +19,38 @@
package org.grails.plugins
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import org.springframework.beans.factory.config.RuntimeBeanReference
import org.springframework.core.env.StandardEnvironment
+import org.springframework.jdbc.datasource.DataSourceTransactionManager
import grails.plugins.GrailsPlugin
import grails.plugins.GrailsPluginManager
+import grails.util.BuildSettings
import grails.web.servlet.plugins.GrailsWebPluginManager
import org.apache.grails.core.plugins.DefaultPluginDiscovery
import org.grails.config.PropertySourcesConfig
+import org.grails.core.support.ClassEditor
+import org.grails.beans.support.PropertiesEditor
+import org.grails.commons.test.AbstractGrailsMockTests
import
org.grails.spring.aop.autoproxy.GroovyAwareAspectJAwareAdvisorAutoProxyCreator
import
org.grails.spring.aop.autoproxy.GroovyAwareInfrastructureAdvisorAutoProxyCreator
import org.grails.web.servlet.context.support.WebRuntimeSpringConfiguration
-import org.grails.commons.test.AbstractGrailsMockTests
-import org.springframework.jdbc.datasource.DataSourceTransactionManager
-import org.springframework.beans.factory.config.RuntimeBeanReference
class CoreGrailsPluginTests extends AbstractGrailsMockTests {
+ @BeforeEach
+ void setUpTest() throws Exception {
+ super.setUp()
+ }
+
+ @AfterEach
+ void tearDownTest() throws Exception {
+ super.tearDown()
+ }
+
void testComponentScan() {
Review Comment:
This test class is executed via JUnit Jupiter (see
grails-test-suite-uber/build.gradle useJUnitPlatform + junit-jupiter-engine),
but test methods without `@Test` are not discovered. Add `@Test` to
testComponentScan() (and similarly to the other remaining test* methods) so
they continue running.
This issue also appears on line 93 of the same file.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
