Skip to content

Commit 4d2d051

Browse files
tulinkryVladimir Kotal
tulinkry
authored and
Vladimir Kotal
committed
fixing authorization layer tests
1 parent dfeaf33 commit 4d2d051

File tree

5 files changed

+53
-46
lines changed

5 files changed

+53
-46
lines changed

‎opengrok-indexer/src/main/java/org/opengrok/indexer/authorization/AuthorizationFramework.java

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ public IAuthorizationPlugin handleLoadClass(String classname) {
414414
try {
415415
return loadClass(classname);
416416
} catch (ClassNotFoundException ex) {
417-
LOGGER.log(Level.WARNING, String.format("Class \"%s\" was not found: ", classname), ex);
417+
LOGGER.log(Level.WARNING, String.format("Class \"%s\" was not found", classname), ex);
418418
} catch (SecurityException ex) {
419419
LOGGER.log(Level.WARNING, String.format("Class \"%s\" was found but it is placed in prohibited package: ", classname), ex);
420420
} catch (InstantiationException ex) {
@@ -533,11 +533,15 @@ private void loadClassFiles(AuthorizationStack stack, List<File> classfiles) {
533533
private void loadJarFiles(AuthorizationStack stack, List<File> jarfiles) {
534534
IAuthorizationPlugin pf;
535535

536+
536537
for (File file : jarfiles) {
537538
try (JarFile jar = new JarFile(file)) {
538539
Enumeration<JarEntry> entries = jar.entries();
539540
while (entries.hasMoreElements()) {
540541
JarEntry entry = entries.nextElement();
542+
if (!entry.getName().endsWith(".class")) {
543+
continue;
544+
}
541545
String classname = getClassName(entry);
542546
if (!entry.getName().endsWith(".class") || classname.isEmpty()) {
543547
continue;
@@ -559,12 +563,15 @@ private void loadJarFiles(AuthorizationStack stack, List<File> jarfiles) {
559563
private String getClassName(File f) {
560564
String classname = f.getAbsolutePath().substring(pluginDirectory.getAbsolutePath().length() + 1, f.getAbsolutePath().length());
561565
classname = classname.replace(File.separatorChar, '.'); // convert to package name
566+
// no need to check for the index from lastIndexOf because we're in a branch
567+
// where we expect the .class suffix
562568
classname = classname.substring(0, classname.lastIndexOf('.')); // strip .class
563569
return classname;
564570
}
565571

566572
private String getClassName(JarEntry f) {
567-
String classname = f.getName().replace(File.separatorChar, '.'); // convert to package name
573+
// java jar always uses / as separator
574+
String classname = f.getName().replace('/', '.'); // convert to package name
568575
return classname.substring(0, classname.lastIndexOf('.')); // strip .class
569576
}
570577

@@ -616,7 +623,7 @@ public Object run() {
616623
} else {
617624
newLocalStack = this.newStack.clone();
618625
}
619-
626+
620627
// Load all other possible plugin classes.
621628
if (isLoadClassesEnabled()) {
622629
loadClassFiles(newLocalStack, IOUtils.listFilesRec(pluginDirectory, ".class"));
@@ -649,7 +656,7 @@ public Object run() {
649656

650657
Statistics stats = RuntimeEnvironment.getInstance().getStatistics();
651658
stats.addRequest("authorization_stack_reload");
652-
659+
653660
// clean the old stack
654661
removeAll(oldStack);
655662
oldStack = null;
@@ -682,7 +689,7 @@ private void increasePluginVersion() {
682689
*
683690
* Assumes the {@code lock} is held for reading.
684691
*
685-
* @param req the request
692+
* @param session the request session
686693
* @return true if it is; false otherwise
687694
*/
688695
private boolean isSessionInvalid(HttpSession session) {
@@ -738,8 +745,9 @@ private boolean isSessionInvalid(HttpSession session) {
738745
*
739746
* @param request request object
740747
* @param cache cache
741-
* @param name name
742-
* @param predicate predicate
748+
* @param entity entity with name
749+
* @param pluginPredicate predicate to determine the plugin's decision for the request
750+
* @param skippingPredicate predicate to determine if the plugin should be skipped for this request
743751
* @return true if yes
744752
*
745753
* @see RuntimeEnvironment#getPluginStack()

‎opengrok-indexer/src/main/java/org/opengrok/indexer/authorization/AuthorizationPluginClassLoader.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ public boolean accept(File dir, String name) {
8888

8989
for (File f : jars) {
9090
try (JarFile jar = new JarFile(f)) {
91-
String filename = classname.replace('.', File.separatorChar) + ".class";
91+
// jar files always use / separator
92+
String filename = classname.replace('.', '/') + ".class";
9293
JarEntry entry = (JarEntry) jar.getEntry(filename);
9394
if (entry != null && entry.getName().endsWith(".class")) {
9495
try (InputStream is = jar.getInputStream(entry)) {
@@ -105,7 +106,7 @@ public boolean accept(File dir, String name) {
105106
} catch (IOException ex) {
106107
LOGGER.log(Level.SEVERE, "Loading class threw an exception:", ex);
107108
} catch (Throwable ex) {
108-
LOGGER.log(Level.SEVERE, "Loading class threw an unknown exception:", ex);
109+
LOGGER.log(Level.SEVERE, "Loading class threw an unknown exception", ex);
109110
}
110111
}
111112
throw new ClassNotFoundException("Class \"" + classname + "\" could not be found");

‎opengrok-indexer/src/test/java/org/opengrok/indexer/authorization/AuthorizationFrameworkReloadTest.java

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,16 @@
2222
*/
2323
package org.opengrok.indexer.authorization;
2424

25-
import java.io.File;
26-
import java.net.URL;
27-
import javax.servlet.http.HttpSession;
2825
import static org.junit.Assert.assertEquals;
2926
import static org.junit.Assert.assertFalse;
3027
import static org.junit.Assert.assertNotNull;
3128
import static org.junit.Assert.assertNull;
3229
import static org.junit.Assert.assertTrue;
30+
31+
import java.io.File;
32+
import java.net.URISyntaxException;
33+
import java.nio.file.Paths;
34+
import javax.servlet.http.HttpSession;
3335
import org.junit.Test;
3436
import org.opengrok.indexer.configuration.Project;
3537
import org.opengrok.indexer.configuration.RuntimeEnvironment;
@@ -38,20 +40,18 @@
3840

3941
/**
4042
* Test behavior of AuthorizationFramework {@code reload()} w.r.t. HTTP sessions.
41-
*
43+
*
4244
* @author Vladimir Kotal
4345
*/
4446
public class AuthorizationFrameworkReloadTest {
45-
47+
4648
private final File pluginDirectory;
47-
4849
volatile boolean runThread;
49-
50-
public AuthorizationFrameworkReloadTest() {
51-
URL resource = AuthorizationFrameworkReloadTest.class.getResource("/authorization/plugins/testplugins.jar");
52-
pluginDirectory = new File(resource.getFile()).getParentFile();
50+
51+
public AuthorizationFrameworkReloadTest() throws URISyntaxException {
52+
pluginDirectory = Paths.get(getClass().getResource("/authorization/plugins/testplugins.jar").toURI()).toFile().getParentFile();
5353
}
54-
54+
5555
/**
5656
* After {@code reload()} the session attributes should be invalidated.
5757
* It is assumed that invalidation of HttpSession objects means that all
@@ -64,18 +64,18 @@ public void testReloadSimple() {
6464
framework.setLoadClasses(false); // to avoid noise when loading classes of other tests
6565
framework.reload();
6666
Statistics stats = RuntimeEnvironment.getInstance().getStatistics();
67-
67+
6868
// Ensure the framework was setup correctly.
6969
assertNotNull(framework.getPluginDirectory());
7070
assertEquals(pluginDirectory, framework.getPluginDirectory());
71-
71+
7272
// Create pre-requisite objects - mainly the HTTP session with attribute.
7373
Project p = new Project("project" + Math.random());
7474
HttpSession session = req.getSession();
7575
String attrName = "foo";
7676
session.setAttribute(attrName, "bar");
7777
assertNotNull(session.getAttribute(attrName));
78-
78+
7979
// Reload the framework to increment the plugin generation version.
8080
framework.reload();
8181
// Let the framework check the request. This should invalidate the session
@@ -86,35 +86,35 @@ public void testReloadSimple() {
8686
// Verify that the session no longer has the attribute.
8787
assertNull(session.getAttribute(attrName));
8888
}
89-
89+
9090
/**
9191
* Sort of a stress test - call isAllowed() and reload() in parallel.
9292
* This might uncover any snags with locking within AuthorizationFramework.
9393
*/
9494
@Test
95-
public void testReloadCycle() {
95+
public void testReloadCycle() throws URISyntaxException {
9696
Statistics stats = RuntimeEnvironment.getInstance().getStatistics();
9797
Long reloads;
9898
String projectName = "project" + Math.random();
99-
99+
100100
// Create authorization stack for single project.
101101
AuthorizationStack stack = new AuthorizationStack(AuthControlFlag.REQUIRED,
102102
"stack for project " + projectName);
103103
assertNotNull(stack);
104-
stack.add(new AuthorizationPlugin(AuthControlFlag.REQUIRED,
104+
stack.add(new AuthorizationPlugin(AuthControlFlag.REQUIRED,
105105
"opengrok.auth.plugin.FalsePlugin"));
106106
stack.setForProjects(projectName);
107-
AuthorizationFramework framework =
107+
AuthorizationFramework framework =
108108
new AuthorizationFramework(pluginDirectory.getPath(), stack);
109109
framework.setLoadClasses(false); // to avoid noise when loading classes of other tests
110110
framework.reload();
111-
111+
112112
// Perform simple sanity check before long run is entered. If this fails,
113113
// it will be waste of time to continue with the test.
114114
Project p = new Project(projectName);
115115
DummyHttpServletRequest req = new DummyHttpServletRequest();
116116
assertFalse(framework.isAllowed(req, p));
117-
117+
118118
// Create a thread that does reload() every now and then.
119119
runThread = true;
120120
final int maxReloadSleep = 10;
@@ -131,7 +131,7 @@ public void run() {
131131
}
132132
});
133133
t.start();
134-
134+
135135
reloads = stats.getRequest("authorization_stack_reload");
136136
assertNotNull(reloads);
137137
// Process number or requests and check that framework decision is consistent.
@@ -144,28 +144,27 @@ public void run() {
144144
} catch (InterruptedException ex) {
145145
}
146146
}
147-
147+
148148
try {
149149
// Terminate the thread.
150150
runThread = false;
151151
t.join();
152152
} catch (InterruptedException ex) {
153153
}
154-
154+
155155
// Double check that at least one reload() was done.
156156
reloads = stats.getRequest("authorization_stack_reload") - reloads;
157-
System.out.println("number of reloads: " + reloads);
158157
assertTrue(reloads > 0);
159158
}
160-
159+
161160
@Test
162161
public void testSetLoadClasses() {
163162
AuthorizationFramework framework = new AuthorizationFramework();
164163
assertTrue(framework.isLoadClassesEnabled());
165164
framework.setLoadClasses(false);
166165
assertFalse(framework.isLoadClassesEnabled());
167166
}
168-
167+
169168
@Test
170169
public void testSetLoadJars() {
171170
AuthorizationFramework framework = new AuthorizationFramework();

‎opengrok-indexer/src/test/java/org/opengrok/indexer/authorization/AuthorizationPluginClassLoaderTest.java

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,21 @@
2424

2525
import java.io.File;
2626
import java.net.URISyntaxException;
27-
import java.net.URL;
2827
import java.nio.file.Paths;
29-
28+
import java.util.logging.Level;
3029
import org.junit.Assert;
3130
import org.junit.Test;
3231
import org.opengrok.indexer.configuration.Group;
3332
import org.opengrok.indexer.configuration.Project;
33+
import org.opengrok.indexer.logger.LoggerUtil;
3434
import org.opengrok.indexer.web.DummyHttpServletRequest;
3535

3636
public class AuthorizationPluginClassLoaderTest {
3737

3838
private final File pluginDirectory;
3939

4040
public AuthorizationPluginClassLoaderTest() throws URISyntaxException {
41-
URL resource = AuthorizationPluginClassLoaderTest.class.getResource("/authorization/plugins/testplugins.jar");
42-
pluginDirectory = Paths.get(resource.toURI()).toFile().getParentFile();
41+
pluginDirectory = Paths.get(getClass().getResource("/authorization/plugins/testplugins.jar").toURI()).toFile().getParentFile();
4342
Assert.assertTrue(pluginDirectory.isDirectory());
4443
}
4544

@@ -142,7 +141,7 @@ public void testNonExistingPlugin() {
142141
AuthorizationPluginClassLoader instance
143142
= new AuthorizationPluginClassLoader(pluginDirectory);
144143

145-
Class clazz = loadClass(instance, "org.sample.plugin.NoPlugin", true);
144+
loadClass(instance, "org.sample.plugin.NoPlugin", true);
146145
}
147146

148147
@Test
@@ -212,15 +211,15 @@ private Class loadClass(AuthorizationPluginClassLoader loader, String name, bool
212211
}
213212
} catch (ClassNotFoundException ex) {
214213
if (!shouldFail) {
215-
Assert.fail("Should not produce ClassNotFoundException");
214+
Assert.fail(String.format("Should not produce ClassNotFoundException: %s", ex.getLocalizedMessage()));
216215
}
217216
} catch (SecurityException ex) {
218217
if (!shouldFail) {
219-
Assert.fail("Should not produce SecurityException");
218+
Assert.fail(String.format("Should not produce SecurityException: %s", ex.getLocalizedMessage()));
220219
}
221220
} catch (Exception ex) {
222221
if (!shouldFail) {
223-
Assert.fail("Should not produce any exception");
222+
Assert.fail(String.format("Should not produce any exception: %s", ex.getLocalizedMessage()));
224223
}
225224
}
226225
return clazz;

‎pom.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,8 @@ Portions Copyright (c) 2018, Chris Fraire <[email protected]>.
206206
<groupId>org.apache.maven.plugins</groupId>
207207
<artifactId>maven-surefire-plugin</artifactId>
208208
<configuration>
209-
<forkCount>1</forkCount>
210-
<reuseForks>false</reuseForks>
209+
<forkCount>1</forkCount>
210+
<reuseForks>false</reuseForks>
211211
</configuration>
212212
<version>2.22.0</version>
213213
<executions>

0 commit comments

Comments
 (0)