Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

NEXUS-4955 - expand the prefix when it is too short #349

Merged
merged 3 commits into from

3 participants

@velo

This issue was reported on user list.

Basically File.createTempFile() fail if prefix is too short, what may happen when the file name we are uploading is too short.

I just append System.nanotime() to it, but we can append anything else.

@cstamas cstamas commented on the diff
...uration/application/RepositoryDependentException.java
@@ -1,3 +1,15 @@
+/**
+ * Sonatype Nexus (TM) Open Source Version
+ * Copyright (c) 2007-2012 Sonatype, Inc.
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
+ *
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
+ *
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
+ */
package org.sonatype.nexus.configuration.application;
@cstamas Owner
cstamas added a note

? What's this?

@velo
velo added a note

A missing header...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...atype/nexus/proxy/storage/local/fs/DefaultFSPeer.java
@@ -328,6 +333,18 @@ protected File getHiddenTarget( final File target, final StorageItem item )
}
}
+ private String expandPrefix( String prefix )
@cstamas Owner
cstamas added a note

Never leave any chance for shit to hit the fan.

Here, you would have it, if -- even if there is a minimal chance, but there IS a chance -- to deploy file "id" and have nanoTime as 12345... and then in very next moment deploy of file "id12345..."... kaboom, collision might happen.

Why not always prefix with same content?

Even something like return "aaa" + prefix; would work, as with this you effectively "translate" the space (which is a "space of possible file names" here) to have it's border above minimum required (minimum name length of 3 characters), but since translation is "rigid motion", you preserved uniqueness too, unlike the solution in this pull request (and hence, the minimum chance to something smelly hit the fan is present).

@velo
velo added a note

So you are saying collision will happen 100% of the times name is bigger then 3 chars? It doesn't make sense.

I will use the constant instead of nanoTime.

@cstamas Owner
cstamas added a note

No, I am saying there is room for collision.

One deploys file "id", and nano at the moment is "12345". So, the prefix you create is "id12345".
In same moment, someone else deploys file named "id12345". Here, you do not prefix, since name is longer than 3.

But the strings returned by new expandPrefix method are actually the same.

@velo
velo added a note

So when there is no nano on the mix there will be collision....

@cstamas Owner
cstamas added a note

No, it is the condition that makes it possible. If you append unconditionally with a constant, you make it impossible to produce a collision.

@velo
velo added a note

ok, then I will remove the if.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cstamas
Owner

:beer:

@adreghiciu
Owner

+1

@velo velo merged commit b034c55 into master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
12 .../main/java/org/sonatype/nexus/configuration/application/RepositoryDependentException.java
@@ -1,3 +1,15 @@
+/**
+ * Sonatype Nexus (TM) Open Source Version
+ * Copyright (c) 2007-2012 Sonatype, Inc.
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
+ *
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
+ *
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
+ */
package org.sonatype.nexus.configuration.application;
@cstamas Owner
cstamas added a note

? What's this?

@velo
velo added a note

A missing header...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
import static java.lang.String.format;
View
9 nexus/nexus-proxy/src/main/java/org/sonatype/nexus/proxy/storage/local/fs/DefaultFSPeer.java
@@ -12,6 +12,8 @@
*/
package org.sonatype.nexus.proxy.storage.local.fs;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import java.io.File;
import java.io.FileFilter;
import java.io.FileOutputStream;
@@ -54,6 +56,8 @@
{
private static final String HIDDEN_TARGET_SUFFIX = ".nx-upload";
+ private static final String APPENDIX = "nx-tmp";
+
public boolean isReachable( Repository repository, ResourceStoreRequest request, File target )
throws LocalStorageException
{
@@ -318,9 +322,12 @@ else if ( target.isFile() )
protected File getHiddenTarget( final File target, final StorageItem item )
throws LocalStorageException
{
+ checkNotNull( target );
+
try
{
- return File.createTempFile( target.getName(), HIDDEN_TARGET_SUFFIX, target.getParentFile() );
+ // NEXUS-4955 add APPENDIX to make sure prefix is bigger the 3 chars
+ return File.createTempFile( target.getName() + APPENDIX, HIDDEN_TARGET_SUFFIX, target.getParentFile() );
}
catch ( IOException e )
{
View
39 ...exus-proxy/src/test/java/org/sonatype/nexus/proxy/storage/local/fs/DefaultFSPeerTest.java
@@ -0,0 +1,39 @@
+/**
+ * Sonatype Nexus (TM) Open Source Version
+ * Copyright (c) 2007-2012 Sonatype, Inc.
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
+ *
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
+ *
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
+ */
+package org.sonatype.nexus.proxy.storage.local.fs;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.sonatype.sisu.litmus.testsupport.hamcrest.FileMatchers.exists;
+
+import java.io.File;
+
+import org.junit.Test;
+
+public class DefaultFSPeerTest
+{
+
+ @Test
+ public void testGetHiddenTarget()
+ throws Exception
+ {
+ File base = new File( "target/a/b/c/d" );
+ base.getParentFile().mkdirs();
+ base.createNewFile();
+
+ File created = new DefaultFSPeer().getHiddenTarget( base, null );
+ assertThat( created, notNullValue() );
+ assertThat( created, exists() );
+ }
+
+}
View
1  nexus/nexus-test-harness/nexus-test-harness-its/resources/nexus4955/files/content.txt
@@ -0,0 +1 @@
+any content
View
37 ...test/java/org/sonatype/nexus/integrationtests/nexus4955/NEXUS4955UploadWOExtensionIT.java
@@ -0,0 +1,37 @@
+/**
+ * Sonatype Nexus (TM) Open Source Version
+ * Copyright (c) 2007-2012 Sonatype, Inc.
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
+ *
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
+ *
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
+ */
+package org.sonatype.nexus.integrationtests.nexus4955;
+
+import java.io.File;
+
+import org.sonatype.nexus.integrationtests.AbstractNexusIntegrationTest;
+import org.testng.annotations.Test;
+
+/**
+ * try to upload (deploy it) a file like "id" (no extension, just file name "id") into repo, nexus will die, it was
+ * reported by user on mailing list
+ *
+ * @author Marvin Froeder ( velo at sonatype.com )
+ */
+public class NEXUS4955UploadWOExtensionIT
+ extends AbstractNexusIntegrationTest
+{
+ @Test
+ public void deploy()
+ throws Exception
+ {
+ File file = getTestFile( "content.txt" );
+ getDeployUtils().deployWithWagon( "http", getRepositoryUrl( REPO_TEST_HARNESS_REPO ), file,
+ "nxcm4955/test/1.0/id" );
+ }
+}
Something went wrong with that request. Please try again.