Skip to content

Commit 787b061

Browse files
committed
Improved better logging and fixed some issues
1 parent b41bb97 commit 787b061

File tree

1 file changed

+15
-4
lines changed

1 file changed

+15
-4
lines changed

core/src/main/java/jenkins/security/csp/AvatarContributor.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,16 @@ public static void allow(@CheckForNull String url) {
9797
public static void allowUrl(@CheckForNull String url) {
9898
AvatarContributor self = ExtensionList.lookupSingleton(AvatarContributor.class);
9999

100-
if (addNormalizedUrl(url, self.allowedSources)) {
101-
LOGGER.log(Level.CONFIG, "Adding allowed avatar URL: {0}", url);
100+
String normalized = normalizeUrl(url);
101+
if (normalized == null) {
102+
LOGGER.log(Level.FINE, "Skipping invalid or unsupported avatar URL: {0}", url);
103+
return;
104+
}
105+
106+
if (self.allowedSources.add(normalized)) {
107+
LOGGER.log(Level.CONFIG, "Adding allowed avatar URL: {0} (normalized: {1})", new Object[] { url, normalized });
108+
} else {
109+
LOGGER.log(Level.FINEST, "Skipped adding duplicate allowed avatar URL: {0} (normalized: {1})", new Object[] { url, normalized });
102110
}
103111
}
104112

@@ -173,7 +181,7 @@ public static String extractDomainFromUrl(@CheckForNull String url) {
173181
* </li>
174182
* <li>
175183
* <strong>Converts IDN to ASCII</strong> —
176-
* Browser internally normalize hostnames to their ASCII representation. Normalizing here ensures Jenkins
184+
* Browsers internally normalize hostnames to their ASCII representation. Normalizing here ensures Jenkins
177185
* generates CSP entries that match browser behavior consistently.
178186
* </li>
179187
* <li>
@@ -182,7 +190,7 @@ public static String extractDomainFromUrl(@CheckForNull String url) {
182190
* </li>
183191
* <li>
184192
* <strong>Removes default ports</strong> —
185-
* Ports 80 (HTTP) and 443 (HTTPS) are removed. Removing them avoids duplicate CSP entries that differ only
193+
* Ports 80 (HTTP) and 443 (HTTPS) are removed. Removing them avoids duplicate CSP entries that differ only
186194
* by the presence of a default port.
187195
* </li>
188196
* </ul>
@@ -214,6 +222,8 @@ public static String normalizeUrl(@CheckForNull String url) {
214222
return null;
215223
}
216224

225+
// Used rawAuthority instead of uri.getHost() so we can strictly validate and canonicalize the authority
226+
// component for CSP usage, including explicit handling of IPv6 literals and malformed authorities.
217227
String rawAuthority = uri.getRawAuthority();
218228
if (rawAuthority == null) {
219229
LOGGER.log(Level.FINER, "Ignoring URI without authority: " + url);
@@ -226,6 +236,7 @@ public static String normalizeUrl(@CheckForNull String url) {
226236
if (rawAuthority.startsWith("[")) {
227237
int end = rawAuthority.indexOf(']');
228238
if (end == -1) {
239+
LOGGER.log(Level.FINER, "Ignoring malformed IPv6 authority: {0}", url);
229240
return null;
230241
}
231242
host = rawAuthority.substring(1, end);

0 commit comments

Comments
 (0)