PuzzleServer / mainpuzzleserver

The main repo for the Puzzle Hunt and Puzzleday servers.
MIT License
9 stars 32 forks source link

Feature request: more flexible url structure for customCSSFile #1025

Open jbodner09 opened 3 months ago

jbodner09 commented 3 months ago

Currently, there are a few holes in the ability to link to a CSS file from a puzzle depending on where the CSS file lives. This is the current logic:

string puzzleFilePath = "";
if (Model.Puzzle.CustomURL != null)
{
    puzzleFilePath = @PuzzleHelper.GetFormattedUrl(Model.Puzzle, Model.Event.ID);
}
else if (Model.Puzzle.PuzzleFile != null)
{
    puzzleFilePath = @Model.Puzzle.PuzzleFile.Url.AbsoluteUri;
}
string cssFileName = String.IsNullOrEmpty(Model.Puzzle.CustomCSSFile) ? "" : Model.Puzzle.CustomCSSFile.Trim();
@if (cssFileName.Length > 0)
{
    cssFileName += (!cssFileName.EndsWith(".css")) ? ".css" : "";
    cssFileName = cssFileName.Replace('\\', '/');
    if (cssFileName.Contains('/'))
    {
        if (cssFileName.StartsWith(".") && (embedType > 0))
        {
            // For relative urls, ASSUME THAT THE RULES DESCRIBED ON THE UPLOAD PAGE HAVE BEEN FOLLOWED
            // This means that the leading . will be replaced with the puzzleFilePath variable above, minus the puzzle file itself
            // If the file doesn't start with a ., it's assumed to be a full url that will be used as-is
            cssFileName = puzzleFilePath.Substring(0, puzzleFilePath.LastIndexOf('/')) + cssFileName.Substring(1);
        }
        <link rel="stylesheet" href="@cssFileName" />
    }
    else
    {
        // Otherwise, assume that bare file names are in the shared resources folder 
        // (FileStoragePrefix used to account for different urls between development and production)
        <link rel="stylesheet" href="@(Model.FileStoragePrefix)/resources/@cssFileName" />
    }
}

There are a few problems with this. First, assuming that a string with a / in it will NOT live in the shared resources presents a problem. Right now, that means that EVERYTHING in the shared resources folder that is implicitly addressable via this field must live in the root of that shared resources folder. In other words, there's no way to put these files in a subdirectory in the shared resources folder and have them addressable via this automatic way.

Second, the relative links don't actually appear to be working as intended: using the puzzle's path as the file's path prefix breaks when using "smart" links. Essentially, when you upload files to a puzzle, you get both a "smart" link to the azure site, and the "raw" link to the actual path in azure storage. If you use the raw link as the custom url, everything works fine. If you use the smart link though, it just doesn't work. And given that we WANT people to use smart links since they're event-migration-proof, relative links will likely never be possible to use unless there's a way to instead get the raw link under the hood to use as the file path prefix.

Given how finnicky relative links starting at the puzzle will always be, I suggest just removing that functionality as a 3rd option and just having two options:

This not only still lets authors style their own submission page using their own puzzle files, it also allows subfolders in the shared resources. The only thing that doesn't work with this method, which is what the current relative link implementation was trying to avoid, is that changing the puzzle files will change the location of that CSS file thanks to our autogenerated folders, so that field will not be file-change or event-migration agnostic, so it will still require danger text on the puzzle editing page.

bruceleban commented 3 months ago

Allowing .. in paths is fragile and a bad idea from a security perspective. Accordingly, if browsers see .. they delete the previous path element on the client side. If you're planning to resolve the .. on the server side, that's not an issue.

However, if you are doing this on the server side, another solution is if necessary to add other roots like $global/... for a global shared resource folder, $event/... for an event-specific shared resources folder.