Nemcio/SCM-Manager
1
0
Fork 0
mirror of https://github.com/scm-manager/scm-manager.git synced 2025-12-21 15:59:48 +01:00
Code Issues Packages Projects Releases Activity

Fix permission errors

Browse Source 
The BaseReceivePackFactory re-used the GitReceiveHook. The
problem is, that the hook is not thread safe. Due to the
re-usage, the repository could have been changed during
processing post receive hooks.

With this, the factory will always create a new receive pack.

Pushed-by: Rene Pfeuffer<rene.pfeuffer@cloudogu.com>
Co-authored-by: René Pfeuffer<rene.pfeuffer@cloudogu.com>
Committed-by: René Pfeuffer<rene.pfeuffer@cloudogu.com>
This commit is contained in:
Rene Pfeuffer
2024-08-08 16:27:48 +02:00
parent db23e633f8
commit 27f78d2347
5 changed files with 24 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java
View File

@@ -39,17 +39,19 @@ import sonia.scm.web.GitReceiveHook;
public abstract class BaseReceivePackFactory<T> implements ReceivePackFactory<T> {
private final GitChangesetConverterFactory converterFactory;
private final GitRepositoryHandler handler;
private final GitReceiveHook hook;
private final HookEventFacade hookEventFacade;
private final GitRepositoryConfigStoreProvider storeProvider;
protected BaseReceivePackFactory(GitChangesetConverterFactory converterFactory,
GitRepositoryHandler handler,
HookEventFacade hookEventFacade,
GitRepositoryConfigStoreProvider storeProvider) {
this.converterFactory = converterFactory;
this.handler = handler;
this.hookEventFacade = hookEventFacade;
this.storeProvider = storeProvider;
this.hook = new GitReceiveHook(converterFactory, hookEventFacade, handler);
}
@Override
@@ -57,6 +59,7 @@ public abstract class BaseReceivePackFactory<T> implements ReceivePackFactory<T>
ReceivePack receivePack = createBasicReceivePack(connection, repository);
receivePack.setAllowNonFastForwards(isNonFastForwardAllowed(repository));
GitReceiveHook hook = new GitReceiveHook(converterFactory, hookEventFacade, handler);
receivePack.setPreReceiveHook(hook);
receivePack.setPostReceiveHook(hook);
// apply collecting listener, to be able to check which commits are new

2
scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/CollectingPackParserListener.java
View File

@@ -55,7 +55,7 @@ public class CollectingPackParserListener implements PackParserListener
private Set<ObjectId> newObjectIds;
public CollectingPackParserListener(GitReceiveHook hook) {
private CollectingPackParserListener(GitReceiveHook hook) {
this.hook = hook;
}

7
scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceiveHook.java
View File

@@ -43,7 +43,9 @@ import java.io.IOException;
import java.util.Collection;
import java.util.List;
/**
* This class is <b>not</b> thread safe!
*/
public class GitReceiveHook implements PreReceiveHook, PostReceiveHook {
private static final Logger LOG = LoggerFactory.getLogger(GitReceiveHook.class);
@@ -96,6 +98,9 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook {
GitHookContextProvider context = new GitHookContextProvider(converterFactory, rpack, receiveCommands, repository, repositoryId);
if (type == RepositoryHookType.POST_RECEIVE) {
if (postReceiveContext != null) {
throw new IllegalStateException("Looks like this hook is re-used. This must not happen.");
}
postReceiveContext = context;
} else {
hookEventFacade.handle(repositoryId).fireHookEvent(type, context);

12
scm-plugins/scm-git-plugin/src/test/java/sonia/scm/protocolcommand/git/BaseReceivePackFactoryTest.java
View File

@@ -50,6 +50,8 @@ import java.io.File;
import java.io.IOException;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.sameInstance;
import static org.junit.Assert.*;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -88,8 +90,7 @@ public class BaseReceivePackFactoryTest {
when(handler.getConfig()).thenReturn(gitConfig);
when(handler.getRepositoryId(repository.getConfig())).thenReturn("heart-of-gold");
ReceivePack receivePack = new ReceivePack(repository);
when(wrappedReceivePackFactory.create(request, repository)).thenReturn(receivePack);
when(wrappedReceivePackFactory.create(request, repository)).thenAnswer(invocation -> new ReceivePack(repository));
when(gitRepositoryConfigStoreProvider.getGitRepositoryConfig("heart-of-gold")).thenReturn(gitRepositoryConfig);
@@ -130,4 +131,11 @@ public class BaseReceivePackFactoryTest {
assertFalse(receivePack.isAllowNonFastForwards());
}
@Test
public void shouldNotReUseGitReceiveHook() throws Exception {
ReceivePack receivePack1 = factory.create(request, repository);
ReceivePack receivePack2 = factory.create(request, repository);
assertThat(receivePack1.getPostReceiveHook(), not(sameInstance(receivePack2.getPostReceiveHook())));
}
}