Extensions with period

[asherber]

Thanks for this useful library!

I know this would be a breaking change, but I'd like to suggest that the values in FileFormat.Extension should start with a period, just like the output of Path.GetExtension() -- and like the table in this repo's README.

[neilharvey]

I'm in two minds about this one. It probably does make sense to align with the way that Path.GetExtension works, but is that a good enough reason to create a breaking change?

I'm tempted to leave this issue open for a little while to give a chance to hear some more opinons on this.

[asherber]

I totally understand. Maybe this is something you just want to hang onto until the next major release, rather than making a major release just for this.

Two other possibilities would be a static property, something like FileFormatInspector.ExtensionHasDot, which instructs file formats to include a dot before the extension (defaults to false) or an additional optional extensionHasDot parameter to the FileFormatInspector constructor which accomplishes the same thing.

I'm happy to work on a PR if you like one of those approaches.

[tiesont]

Totally useless comment: I'm ambivalent, either way. As long as I know how it works in any given version, I'm fine with the change. Might be nice to match Path.GetExtension, if for no other reason than maybe it has a history and therefore has set the precedent?

[neilharvey]

@tiesont It's useful to hear that it wouldn't break what your use case, so thanks for the feedback :)

I've thought about this a bit over the weekend, and I'm inclined to change it. Having consistency with the standard libraries would help to make the usage more predicable, so that's probably a good enough reason to change it.

I also had a look at some of the other System.IO APIs, FileInfo.Extension works the same way, and Path.ChangeExtension produces the same regardless of whether the input which helped to convince me.

@asherber Thanks for the suggestion, but to make that work I'd have to make the formats mutable (either at the static or instance level) to make the option work which doesn't really appeal - I prefer having the formats fixed as immutable types (because a format doesn't change). I think what I'd prefer to do is just accept the breaking change and include the period in the definitions of each format.

[kevgeni]

If I may leave an opinion, at this point it's been 2 years, and this library has quite some download in nuget, so i'll err on the side of don't break existing app. This is my example code that will break if it changes. Currently I remove the . from .net to compare it with this library. Is there a way to make it full proof? yes. Is that useful, or is everyone going to make it fullproof when first using it ? I doubt. Modifying the behaviour to add a period to FileSignature might break some code and it's not worth it 2 years later.

string extension = Path.GetExtension(file.FileName).TrimStart('.');
if (_allowedImageType.Select(q => q.Extension).Distinct().Contains(extension, StringComparer.InvariantCultureIgnoreCase) == false)
{
    throw new BadHttpRequestException("Bad file extension");
}