Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filename filter maps #208

Merged
merged 2 commits into from
Oct 10, 2018
Merged

filename filter maps #208

merged 2 commits into from
Oct 10, 2018

Conversation

1-alex98
Copy link
Member

No description provided.

@ghost ghost assigned 1-alex98 Mar 21, 2018
@ghost ghost added the in progress label Mar 21, 2018
@1-alex98 1-alex98 force-pushed the feature/#207-filename-filter-maps branch 4 times, most recently from 67975d0 to e75f15e Compare March 21, 2018 19:23
@1-alex98
Copy link
Member Author

1-alex98 commented May 2, 2018

How do we go about it?

@bukajsytlos
Copy link
Member

Maybe you could use FileNameUtil. If you have windows environment you could run tests. @micheljung was complaining about test failing under windows. But it is working fine in linux env. Maybe regex can be tweaked to support both platforms' filenames

@1-alex98 1-alex98 added ready and removed in progress labels Jul 7, 2018
@1-alex98
Copy link
Member Author

1-alex98 commented Jul 13, 2018

Where should I use FileNameUtil? And which class of FileUtil, can not find any thing helping there

@1-alex98
Copy link
Member Author

Can you comment on lines ?

@1-alex98 1-alex98 force-pushed the feature/#207-filename-filter-maps branch from e75f15e to c395468 Compare July 13, 2018 20:08
@ghost ghost added in progress and removed ready labels Jul 13, 2018
@1-alex98
Copy link
Member Author

The approach here is to just remove faulty characters here on upload

@@ -71,6 +71,8 @@ void uploadMap(byte[] mapData, String mapFilename, Player author, boolean isRank
Assert.notNull(author, "'author' must not be null");
Assert.isTrue(mapData.length > 0, "'mapData' must not be empty");

mapFilename = mapFilename.replaceAll("[^\\w.\\-]", ""); //replacing all characters that are illegal in filenames
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider using FileNameUtil.normalizeFileName method

@1-alex98 1-alex98 force-pushed the feature/#207-filename-filter-maps branch from c395468 to ea39852 Compare September 7, 2018 18:00
@1-alex98
Copy link
Member Author

1-alex98 commented Sep 7, 2018

I did

@1-alex98 1-alex98 added ready and removed in progress labels Sep 7, 2018
@Brutus5000
Copy link
Member

Due to current cases on prod we not only need to check the file name but also remove all non-ascii characters from the map name itself. @axel1200 Could you add this here?

@1-alex98
Copy link
Member Author

1-alex98 commented Sep 9, 2018

Ehmm is that not what I was doing?

@1-alex98 1-alex98 force-pushed the feature/#207-filename-filter-maps branch from ea39852 to b5c59a8 Compare September 9, 2018 15:25
@ghost ghost added in progress and removed ready labels Sep 9, 2018
@1-alex98
Copy link
Member Author

@bukajsytlos I looked into the normalize file name method and it is pretty bad it replaces %| and some other character instead of replacing everything that is not a letter or an underscore. because I could imagine some additional characters that wont get replaced like this and would cause problems again...

@bukajsytlos
Copy link
Member

@axel1200 yeah I take a look at it too. The thing is that it should produce valid filenames from OS stand-point of view. If this does not meet business requirement for map names we could create new name transformation. Not sure which characters should be removed for backward compatibility

@1-alex98
Copy link
Member Author

tab for example should be removed

@1-alex98
Copy link
Member Author

1-alex98 commented Oct 3, 2018

for example today somebody uploaded map with : character in it and broke the vault

@Brutus5000
Copy link
Member

Short update: I am currently working on improvements of this PR

@1-alex98
Copy link
Member Author

1-alex98 commented Oct 6, 2018

Do so

@Brutus5000 Brutus5000 force-pushed the feature/#207-filename-filter-maps branch from f2c68c2 to d6a55ab Compare October 9, 2018 20:42
@Brutus5000 Brutus5000 force-pushed the feature/#207-filename-filter-maps branch from d6a55ab to 9da02aa Compare October 9, 2018 22:08
@micheljung micheljung merged commit 24e46d9 into develop Oct 10, 2018
@Brutus5000 Brutus5000 deleted the feature/#207-filename-filter-maps branch October 27, 2018 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants