Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle javadoc option "--allow-script-in-comments" #1393

Closed
mnlipp opened this issue Feb 11, 2017 · 13 comments
Closed

Handle javadoc option "--allow-script-in-comments" #1393

mnlipp opened this issue Feb 11, 2017 · 13 comments

Comments

@mnlipp
Copy link

@mnlipp mnlipp commented Feb 11, 2017

Context

Java 8 update 121 has introduced a new Javadoc option "--allow-script-in-comments" that must be specified if the HTML to be inserted e.g. as "bottom" contains JavaScript.

It is possible to add this to the javadoc task as options.addBooleanOption("-allow-script-in-comments", true). The problem is that this will only work if the java version used for the build is actually greater than Java 8 update 121. Which leads to this workaround:

    def javaVersionMatcher =
        org.gradle.internal.jvm.Jvm.current() =~
        /(\d+)\.(\d+)\.(\d+)_(\d+) .*/
    if (javaVersionMatcher.matches()) {
        def major = javaVersionMatcher.group(1).toInteger()
        def minor = javaVersionMatcher.group(2).toInteger()
        def patch = javaVersionMatcher.group(3).toInteger()
        def build = javaVersionMatcher.group(4).toInteger()
        if (major > 1
            || major == 1 && minor > 8
            || major == 1 && minor == 8 && patch > 0
            || major == 1 && minor == 8 && patch == 0 && build >= 121) {
            options.addBooleanOption("-allow-script-in-comments", true)
        }
    }

(Note that I love gradle for allowing me to work around the problem!)

(16.2.2017: see simpler solution below.)

Expected Behavior

On the long run, the behavior from above should be implemented as some "special options" for the javadoc task.

@eriwen
Copy link
Member

@eriwen eriwen commented Feb 13, 2017

Yikes, what a workaround. I'm curious if there's a less fragile way to handle this (for example, this doesn't support Java 9+) and I'd encourage someone to find it and submit a PR for it.

@bmuschko
Copy link
Contributor

@bmuschko bmuschko commented Feb 15, 2017

@mnlipp What happens if you do provide the option and the JDK version does not support it? Does the task fail?

@mnlipp
Copy link
Author

@mnlipp mnlipp commented Feb 16, 2017

@bmuschko I haven't got an older JDK version installed, but reportedly it does.

The option "crept in" with 8u121. From the release notes:

New --allow-script-in-comments option for javadoc
The javadoc tool will now reject any occurrences of JavaScript code in the javadoc documentation comments and command-line options, unless the command-line option, --allow-script-in-comments is specified.

With the --allow-script-in-comments option, the javadoc tool will preserve JavaScript code in documentation comments and command-line options. An error will be given by the javadoc tool if JavaScript code is found and the command-line option is not set.
JDK-8138725 (not public)

As the issue is not public, it's hard to say why they have introduced the option. Either you do need JavaScript in the doc or you don't. If you do, you now have to pass that additional parameter. I cannot see a gain in security or anything here, it's just a d... nuisance.

Ideally, we'd parse the output from javadoc --help to find out if the option is supported. Bad news: --allow-script-in-comments doesn't appear in this output (maybe this is why the option has two leading dashes). Maybe you can get the information from javax.tools.OptionChecker. Didn't think of this before. I'll check it.

@mnlipp
Copy link
Author

@mnlipp mnlipp commented Feb 16, 2017

Okay, should have thought of this before, much simpler solution:

    if (javax.tools.ToolProvider.getSystemDocumentationTool()
        .isSupportedOption("--allow-script-in-comments")) {
        options.addBooleanOption("-allow-script-in-comments", true)
    }

This could easily be made the basis for a general, future proof solution: check each parameter in the javadoc task before actually adding it and report unsupported parameters specified in the build script as a warning (not as an error, that would just propagate the problem). Regarding --allow-script-in-comments this would result in a warning if your java version is older than 8u121. But I think this is acceptable.

@eriwen
Copy link
Member

@eriwen eriwen commented Feb 16, 2017

@mnlipp Nice, much simpler indeed. Pull request?

@trustin
Copy link

@trustin trustin commented Mar 2, 2017

It seems like the code snippet @mnlipp provided doesn't work. isSupportedOption('--allow-script-in-comments') returns -1 regardless it's pre-121 or not. I ended up with the following hack:

        // Add --allow-script-in-comments if available (since 1.8.0_121)
        try {
            if (Class.forName('com.sun.tools.doclets.formats.html.ConfigurationImpl')
                     .newInstance().optionLength('--allow-script-in-comments') > 0) {
                addBooleanOption('-allow-script-in-comments').value = true
            }
        } catch (ignored) {}

@mnlipp
Copy link
Author

@mnlipp mnlipp commented Mar 2, 2017

Indeed. And my code is missing the comparison with zero: "... .isSupportedOption(...) == 0". (That's why I don't like weakly typed languages, wouldn't have happened with Java!)

Anyway, in case somebody gets here while searching around, I submitted a bug report.

@lukaseder
Copy link

@lukaseder lukaseder commented May 30, 2017

I've run into the same issue with the Javadoc Maven plugin. I'm working around it by post-processing the generated Javadoc, as I've documented here:
https://issues.apache.org/jira/browse/MJAVADOC-481?focusedCommentId=16029488&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16029488

Hope this helps for the time being...

@oehme oehme removed the help-wanted label Sep 20, 2017
ionparticle added a commit to ubc/DSpace that referenced this issue Feb 20, 2018
JavaScript in comments used to be fine, but is now rejected by default
with Java 8u121. Need to add the --allow-script-in-comments option for
javadoc to proceed.

gradle/gradle#1393 (comment)
ionparticle added a commit to ubc/DSpace that referenced this issue Mar 1, 2018
JavaScript in comments used to be fine, but is now rejected by default
with Java 8u121. Need to add the --allow-script-in-comments option for
javadoc to proceed.

gradle/gradle#1393 (comment)
@ljacomet ljacomet added the @jvm label Feb 18, 2020
@stale
Copy link

@stale stale bot commented Feb 17, 2021

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. If you're interested in how we try to keep the backlog in a healthy state, please read our blog post on how we refine our backlog. If you feel this is something you could contribute, please have a look at our Contributor Guide. Thank you for your contribution.

@stale stale bot added the stale label Feb 17, 2021
@stale
Copy link

@stale stale bot commented Mar 11, 2021

This issue has been automatically closed due to inactivity. If you can reproduce this on a recent version of Gradle or if you have a good use case for this feature, please feel free to reopen the issue with steps to reproduce, a quick explanation of your use case or a high-quality pull request.

@stale stale bot closed this Mar 11, 2021
@ddimtirov
Copy link

@ddimtirov ddimtirov commented Jun 22, 2021

#1393 (comment) breaks with Java 11 as the class does not exist.

The original solution works, but relies on internal API.

@trustin
Copy link

@trustin trustin commented Jun 22, 2021

@ddimtirov Here's the workaround I'm using right now:

// Add --allow-script-in-comments if available (since 1.8.0_121)
try {
    if (JavaVersion.current() >= JavaVersion.VERSION_1_9) {
        addBooleanOption('-allow-script-in-comments').value = true
    } else {
        if (Class.forName('com.sun.tools.doclets.formats.html.ConfigurationImpl')
                .newInstance().optionLength('--allow-script-in-comments') > 0) {
            addBooleanOption('-allow-script-in-comments').value = true
        }
    }
} catch (ignored) {}

(Copied from https://github.com/line/gradle-scripts/blob/38f075ed72fe99af5ad146c298b929551fff4254/lib/java-javadoc.gradle#L308-L318)

@ddimtirov
Copy link

@ddimtirov ddimtirov commented Jun 22, 2021

For what it's worth, here is mine :-)

    def javaVersion = System.getProperty("java.version")
    def versionMatcher = javaVersion =~ /(\d+)\.(\d+)\.(\d+)(?:_(\d+))?/
    if (versionMatcher.matches()) {
        def (major, minor, patch, build) = (1..4).collect { versionMatcher.group(it) as Integer }
        if (major > 1 || major == 1 && minor > 8 || major == 1 && minor == 8 && patch > 0 || major == 1 && minor == 8 && patch == 0 && build >= 121) {
            options.addBooleanOption("-allow-script-in-comments", true)
        }
    } else {
        logger.warn("Unrecognized Java version $javaVersion - Javadoc may fail (fix the build).")
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants