Adobe-Consulting-Services / acs-aem-commons

http://adobe-consulting-services.github.io/acs-aem-commons/
Apache License 2.0
451 stars 597 forks source link

A Possible NullPointerException Bug in Version 5.3.0 #2856

Open zhaoyangyingmu opened 2 years ago

zhaoyangyingmu commented 2 years ago

Required Information

Actual Behavior

In version 5.3.0, in method:

com.adobe.acs.commons.rewriter.impl.VersionedClientlibsTransformerFactory.calculateMd5(@Nonnull final HtmlLibrary htmlLibrary, boolean isMinified)

line number: 343 code:

@Nonnull private String calculateMd5(@Nonnull final HtmlLibrary htmlLibrary, boolean isMinified) throws IOException {
    // ...
    try (InputStream input = htmlLibrary.getInputStream(isMinified)) {// htmlLibrary could be null
        // ...
    }
}

htmlLibrary could be null due to invocation BadMd5VersionedClientLibsFilter.doFilter, in which uriInfo.htmlLibrary is passed.

UriInfo uriInfo = getUriInfo(uri, slingRequest);
// ...
md5FromCache = calculateMd5(uriInfo.htmlLibrary, htmlLibraryManager.isMinifyEnabled());
@Nonnull
UriInfo getUriInfo(@Nullable final String uri, @Nonnull SlingHttpServletRequest request) {
    if (uri != null) {
        Matcher matcher;
        // ...
        if (matcher.matches()) {
            // ...
            return new UriInfo(libraryPath + "." + extension, md5, libraryType, htmlLibrary);
        }
    }

    return new UriInfo("", "", null, null);// warning: the fourth parameter is null for htmlLibrary.
}

When matcher.matches() returns false, uriInfo.htmlLibrary will be null, which will induce an npe in your project.

bayrakta commented 1 year ago

Hi zhaoyangyingmu,

I couldn't reproduce the issue on my end. Actually, NPE does not seem to be possible for me, as the UriInfo#cacheKey would be null on a missing htmlLibrary. In such a case, if (uriInfo.cacheKey != null) would yield false and calculateMd5 on line 472 would not be reached. If the issue is reproducible on your end, can you please provide a stacktrace and the url etc that you use, so that I can reproduce the issue on my end? Otherwise, I would suggest closing the issue.

Cheers, Yunus