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

Merged
merged 3 commits into from Mar 26, 2012

Conversation

Projects
None yet
3 participants
Contributor

velo commented Mar 21, 2012

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 Mar 21, 2012

...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

cstamas Mar 21, 2012

Contributor

? What's this?

@velo

velo Mar 21, 2012

Contributor

A missing header...

@cstamas cstamas and 1 other commented on an outdated diff Mar 21, 2012

...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

cstamas Mar 21, 2012

Contributor

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 Mar 22, 2012

Contributor

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

cstamas Mar 22, 2012

Contributor

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 Mar 22, 2012

Contributor

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

@cstamas

cstamas Mar 22, 2012

Contributor

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 Mar 22, 2012

Contributor

ok, then I will remove the if.

Contributor

cstamas commented Mar 22, 2012

🍺

Contributor

adreghiciu commented Mar 26, 2012

+1

@velo velo added a commit that referenced this pull request Mar 26, 2012

@velo velo Merge pull request #349 from sonatype/NEXUS-4955-temp-creation
NEXUS-4955 - expand the prefix when it is too short
b034c55

@velo velo merged commit b034c55 into master Mar 26, 2012

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