Skip to content

Commit dbbbc5b

Browse files
gerlowskijagerlowskija
committed
Fix 'path' and 'permission' related NPEs
Fixes another lingering source of NPEs in v1 and v2 requests.
1 parent e79fc77 commit dbbbc5b

10 files changed

Lines changed: 55 additions & 18 deletions

‎solr/core/src/java/org/apache/solr/api/V2HttpCall.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,8 @@ public void call(SolrQueryRequest req, SolrQueryResponse rsp) {
170170
if (action == REMOTEPROXY) {
171171
action = ADMIN_OR_REMOTEPROXY;
172172
coreUrl = coreUrl.replace("/solr/", "/solr/____v2/c/");
173-
this.path = path = path.substring(prefix.length() + collectionName.length() + 2);
173+
normalizeAndSetPath(path.substring(prefix.length() + collectionName.length() + 2));
174+
path = this.path;
174175
return;
175176
}
176177
}
@@ -188,7 +189,8 @@ public void call(SolrQueryRequest req, SolrQueryResponse rsp) {
188189
}
189190

190191
Thread.currentThread().setContextClassLoader(core.getResourceLoader().getClassLoader());
191-
this.path = path = path.substring(prefix.length() + pathSegments.get(1).length() + 2);
192+
normalizeAndSetPath(path.substring(prefix.length() + pathSegments.get(1).length() + 2));
193+
path = this.path;
192194
// Core-level API, so populate "collection" template val
193195
parts.put(COLLECTION_PROP, origCorename);
194196
Api apiInfo = getApiInfo(core.getRequestHandlers(), path, req.getMethod(), fullPath, parts);

‎solr/core/src/java/org/apache/solr/handler/SchemaHandler.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
118118
handleGET(req, rsp);
119119
break;
120120
default:
121-
throw new SolrException(
122-
SolrException.ErrorCode.BAD_REQUEST, "Unexpected HTTP method: " + httpMethod);
121+
throw getUnexpectedHttpMethodException(httpMethod.name());
123122
}
124123
}
125124

@@ -133,10 +132,16 @@ public PermissionNameProvider.Name getPermissionName(AuthorizationContext ctx) {
133132
case "POST":
134133
return PermissionNameProvider.Name.SCHEMA_EDIT_PERM;
135134
default:
136-
return null;
135+
throw getUnexpectedHttpMethodException(ctx.getHttpMethod());
137136
}
138137
}
139138

139+
public static SolrException getUnexpectedHttpMethodException(String methodName)
140+
throws SolrException {
141+
return new SolrException(
142+
SolrException.ErrorCode.BAD_REQUEST, "Unexpected HTTP method: " + methodName);
143+
}
144+
140145
private void handleGET(SolrQueryRequest req, SolrQueryResponse rsp) {
141146
try {
142147
String path = (String) req.getContext().get("path");

‎solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
154154
command.handleGET();
155155
break;
156156
default:
157-
throw new SolrException(
158-
SolrException.ErrorCode.BAD_REQUEST, "Unexpected HTTP method: " + httpMethod);
157+
throw SchemaHandler.getUnexpectedHttpMethodException(httpMethod.name());
159158
}
160159
}
161160

@@ -964,7 +963,7 @@ public Name getPermissionName(AuthorizationContext ctx) {
964963
case "POST":
965964
return Name.CONFIG_EDIT_PERM;
966965
default:
967-
return null;
966+
throw SchemaHandler.getUnexpectedHttpMethodException(ctx.getHttpMethod());
968967
}
969968
}
970969

‎solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,9 @@ public PermissionNameProvider.Name getPermissionName(AuthorizationContext ctx) {
243243
if (action == null) return PermissionNameProvider.Name.COLL_READ_PERM;
244244
CollectionParams.CollectionAction collectionAction =
245245
CollectionParams.CollectionAction.get(action);
246-
if (collectionAction == null) return null;
246+
if (collectionAction == null) {
247+
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Unknown action: " + action);
248+
}
247249
return collectionAction.isWrite
248250
? PermissionNameProvider.Name.COLL_EDIT_PERM
249251
: PermissionNameProvider.Name.COLL_READ_PERM;

‎solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ public Name getPermissionName(AuthorizationContext ctx) {
203203
return Name.CONFIG_READ_PERM;
204204
}
205205
}
206-
return null;
206+
207+
throw new SolrException(ErrorCode.BAD_REQUEST, "Required parameter 'action' not provided");
207208
}
208209
}

‎solr/core/src/java/org/apache/solr/handler/admin/InfoHandler.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,9 @@ public Name getPermissionName(AuthorizationContext request) {
190190
if (handler != null) {
191191
return handler.getPermissionName(request);
192192
} else {
193-
return null;
193+
throw new SolrException(
194+
SolrException.ErrorCode.BAD_REQUEST,
195+
"Unable to identify 'info' sub-handler for path " + path);
194196
}
195197
}
196198
}

‎solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandler.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.apache.solr.core.CoreContainer;
4444
import org.apache.solr.handler.RequestHandlerBase;
4545
import org.apache.solr.handler.RequestHandlerUtils;
46+
import org.apache.solr.handler.SchemaHandler;
4647
import org.apache.solr.handler.admin.api.GetAuthenticationConfigAPI;
4748
import org.apache.solr.handler.admin.api.GetAuthorizationConfigAPI;
4849
import org.apache.solr.handler.admin.api.ModifyNoAuthPluginSecurityConfigAPI;
@@ -75,7 +76,7 @@ public PermissionNameProvider.Name getPermissionName(AuthorizationContext ctx) {
7576
case "POST":
7677
return PermissionNameProvider.Name.SECURITY_EDIT_PERM;
7778
default:
78-
return null;
79+
throw SchemaHandler.getUnexpectedHttpMethodException(ctx.getHttpMethod());
7980
}
8081
}
8182

‎solr/core/src/java/org/apache/solr/handler/admin/ZookeeperInfoHandler.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.util.TreeMap;
4141
import java.util.regex.Matcher;
4242
import java.util.regex.Pattern;
43+
import org.apache.commons.lang3.StringUtils;
4344
import org.apache.lucene.util.BytesRef;
4445
import org.apache.solr.cloud.ZkController;
4546
import org.apache.solr.common.SolrException;
@@ -55,6 +56,7 @@
5556
import org.apache.solr.common.params.MapSolrParams;
5657
import org.apache.solr.common.params.SolrParams;
5758
import org.apache.solr.common.util.ContentStream;
59+
import org.apache.solr.common.util.SuppressForbidden;
5860
import org.apache.solr.common.util.Utils;
5961
import org.apache.solr.core.CoreContainer;
6062
import org.apache.solr.handler.RequestHandlerBase;
@@ -105,7 +107,7 @@ public Category getCategory() {
105107
@Override
106108
public Name getPermissionName(AuthorizationContext request) {
107109
var params = request.getParams();
108-
String path = params.get(PATH, "");
110+
String path = normalizePath(params.get(PATH, ""));
109111
String detail = params.get(PARAM_DETAIL, "false");
110112
if ("/security.json".equalsIgnoreCase(path) && "true".equalsIgnoreCase(detail)) {
111113
return Name.SECURITY_READ_PERM;
@@ -425,6 +427,11 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
425427
rsp.getValues().add(RawResponseWriter.CONTENT, printer);
426428
}
427429

430+
@SuppressForbidden(reason = "JDK String class doesn't offer a stripEnd equivalent")
431+
private String normalizePath(String path) {
432+
return StringUtils.stripEnd(path, "/");
433+
}
434+
428435
// --------------------------------------------------------------------------------------
429436
//
430437
// --------------------------------------------------------------------------------------

‎solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPluginBase.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,14 @@
3030
import java.util.HashMap;
3131
import java.util.HashSet;
3232
import java.util.List;
33+
import java.util.Locale;
3334
import java.util.Map;
3435
import java.util.Set;
3536
import java.util.function.Function;
3637
import java.util.stream.Collectors;
3738
import org.apache.solr.api.AnnotatedApi;
3839
import org.apache.solr.api.Api;
40+
import org.apache.solr.common.SolrException;
3941
import org.apache.solr.common.SpecProvider;
4042
import org.apache.solr.common.util.CommandOperation;
4143
import org.apache.solr.common.util.ValidatingJsonMap;
@@ -228,6 +230,16 @@ private boolean predefinedPermissionAppliesToRequest(
228230
return false;
229231
} else {
230232
PermissionNameProvider.Name permissionName = handler.getPermissionName(context);
233+
if (permissionName == null) {
234+
final var errorMessage =
235+
String.format(
236+
Locale.ROOT,
237+
"Unable to find 'predefined' associated with requestHandler [%s] and request [%s %s]",
238+
handler.getClass().getName(),
239+
context.getHttpMethod(),
240+
context.getResource());
241+
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, errorMessage);
242+
}
231243

232244
boolean applies =
233245
permissionName != null && predefinedPermission.name.equals(permissionName.name);

‎solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import java.util.Set;
5050
import java.util.function.Supplier;
5151
import net.jcip.annotations.ThreadSafe;
52+
import org.apache.commons.lang3.StringUtils;
5253
import org.apache.solr.api.ApiBag;
5354
import org.apache.solr.api.V2HttpCall;
5455
import org.apache.solr.client.api.util.SolrVersion;
@@ -153,11 +154,16 @@ public HttpSolrCall(
153154
this.requestType = RequestType.UNKNOWN;
154155
this.userAgentSolrVersion = parseUserAgentSolrVersion();
155156
this.span = Optional.ofNullable(TraceUtils.getSpan(req)).orElse(Span.getInvalid());
156-
this.path = ServletUtils.getPathAfterContext(req);
157+
normalizeAndSetPath(ServletUtils.getPathAfterContext(req));
157158

158159
req.setAttribute(HttpSolrCall.class.getName(), this);
159160
}
160161

162+
@SuppressForbidden(reason = "JDK String class doesn't offer a stripEnd equivalent")
163+
protected void normalizeAndSetPath(String unnormalizedPath) {
164+
this.path = StringUtils.stripEnd(unnormalizedPath, "/");
165+
}
166+
161167
public String getPath() {
162168
return path;
163169
}
@@ -212,7 +218,7 @@ protected void init() throws Exception {
212218
// Try to resolve a Solr core name
213219
core = cores.getCore(origCorename);
214220
if (core != null) {
215-
path = path.substring(idx);
221+
normalizeAndSetPath(path.substring(idx));
216222
} else {
217223
// extra mem barriers, so don't look at this before trying to get core
218224
if (cores.isCoreLoading(origCorename)) {
@@ -221,7 +227,7 @@ protected void init() throws Exception {
221227
// the core may have just finished loading
222228
core = cores.getCore(origCorename);
223229
if (core != null) {
224-
path = path.substring(idx);
230+
normalizeAndSetPath(path.substring(idx));
225231
} else {
226232
if (!cores.isZooKeeperAware()) {
227233
core = cores.getCore("");
@@ -250,14 +256,14 @@ protected void init() throws Exception {
250256
core = getCoreByCollection(collectionName, isPreferLeader);
251257
if (core != null) {
252258
if (idx > 0) {
253-
path = path.substring(idx);
259+
normalizeAndSetPath(path.substring(idx));
254260
}
255261
} else {
256262
// if we couldn't find it locally, look on other nodes
257263
if (idx > 0) {
258264
extractRemotePath(collectionName);
259265
if (action == REMOTEPROXY) {
260-
path = path.substring(idx);
266+
normalizeAndSetPath(path.substring(idx));
261267
return;
262268
}
263269
}

0 commit comments

Comments
 (0)