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

Added unsafe load functions (allows absolute file for special cases) #339

Merged
merged 2 commits into from
Feb 11, 2022

Conversation

zefrenchy
Copy link
Contributor

as per #336

@crow-clang-format
Copy link

--- include/crow/http_response.h	(before formatting)
+++ include/crow/http_response.h	(after formatting)
@@ -220,7 +220,7 @@
             utility::sanitize_filename(path);
             set_static_file_info_unsafe(path);
         }
-        
+
         ///Return a static file as the response body without sanitising the path (use set_static_file_info instead)
         void set_static_file_info_unsafe(std::string path)
         {
--- include/crow/mustache.h	(before formatting)
+++ include/crow/mustache.h	(after formatting)
@@ -636,7 +636,7 @@
             utility::sanitize_filename(filename_sanitized);
             return detail::get_loader_ref()(filename_sanitized);
         }
-        
+
         inline std::string load_text_unsafe(const std::string& filename)
         {
             return detail::get_loader_ref()(filename);
@@ -648,7 +648,7 @@
             utility::sanitize_filename(filename_sanitized);
             return compile(detail::get_loader_ref()(filename_sanitized));
         }
-        
+
         inline template_t load_unsafe(const std::string& filename)
         {
             return compile(detail::get_loader_ref()(filename));

@The-EDev The-EDev linked an issue Feb 10, 2022 that may be closed by this pull request
Copy link
Member

@The-EDev The-EDev left a comment

Choose a reason for hiding this comment

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

Aside from clang-format and my other comment. I think it might be best if you force push your changes. Since the previously closed PR's code is still there.

include/crow/http_response.h Outdated Show resolved Hide resolved
@crow-clang-format
Copy link

--- include/crow/http_response.h	(before formatting)
+++ include/crow/http_response.h	(after formatting)
@@ -220,7 +220,7 @@
             utility::sanitize_filename(path);
             set_static_file_info_unsafe(path);
         }
-        
+
         /// Return a static file as the response body without sanitizing the path (use set_static_file_info instead)
         void set_static_file_info_unsafe(std::string path)
         {
--- include/crow/mustache.h	(before formatting)
+++ include/crow/mustache.h	(after formatting)
@@ -636,7 +636,7 @@
             utility::sanitize_filename(filename_sanitized);
             return detail::get_loader_ref()(filename_sanitized);
         }
-        
+
         inline std::string load_text_unsafe(const std::string& filename)
         {
             return detail::get_loader_ref()(filename);
@@ -648,7 +648,7 @@
             utility::sanitize_filename(filename_sanitized);
             return compile(detail::get_loader_ref()(filename_sanitized));
         }
-        
+
         inline template_t load_unsafe(const std::string& filename)
         {
             return compile(detail::get_loader_ref()(filename));

Copy link
Contributor Author

@zefrenchy zefrenchy left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@zefrenchy
Copy link
Contributor Author

completely new to the GitHub workflow ... in case it's not painfully obvious 😅

@crow-clang-format
Copy link

--- include/crow/http_response.h	(before formatting)
+++ include/crow/http_response.h	(after formatting)
@@ -220,7 +220,7 @@
             utility::sanitize_filename(path);
             set_static_file_info_unsafe(path);
         }
-        
+
         /// Return a static file as the response body without sanitizing the path (use set_static_file_info instead)
         void set_static_file_info_unsafe(std::string path)
         {
--- include/crow/mustache.h	(before formatting)
+++ include/crow/mustache.h	(after formatting)
@@ -636,7 +636,7 @@
             utility::sanitize_filename(filename_sanitized);
             return detail::get_loader_ref()(filename_sanitized);
         }
-        
+
         inline std::string load_text_unsafe(const std::string& filename)
         {
             return detail::get_loader_ref()(filename);
@@ -648,7 +648,7 @@
             utility::sanitize_filename(filename_sanitized);
             return compile(detail::get_loader_ref()(filename_sanitized));
         }
-        
+
         inline template_t load_unsafe(const std::string& filename)
         {
             return compile(detail::get_loader_ref()(filename));

Copy link
Member

@The-EDev The-EDev left a comment

Choose a reason for hiding this comment

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

clang-format is complaining about these spaces being in the text, as it requires that any empty lines have no tabs or spaces.

As for force pushing, I don't know what you're using to manage git, but in the terminal you would:

  1. git reset --soft <commit> with <commit> being the commit right before any of your changes in this PR. (make sure you already have all the changes from github already in your local repo, you can run git pull to make sure before you start this process)
  2. git commit -m "my new commit message" (make sure your files are marked as "to be committed, you can do that by running git status after step 1")
  3. git push -f
    This will remove all the old commits in your git branch (and this PR) and put all your changes in the commit you made in step2 instead.

include/crow/http_response.h Outdated Show resolved Hide resolved
include/crow/mustache.h Outdated Show resolved Hide resolved
include/crow/mustache.h Outdated Show resolved Hide resolved
@zefrenchy
Copy link
Contributor Author

Thanks for the workflow pointers.

@zefrenchy
Copy link
Contributor Author

argh so sorry .. I'm not helping much am I?

@The-EDev
Copy link
Member

don't go hard on yourself, it's just a merge conflict, I can handle it if you'd like

@zefrenchy
Copy link
Contributor Author

You're right ... I give up! take over. I'll try to do better next time if I ever have the chance.

@zefrenchy
Copy link
Contributor Author

no idea where those file perm changes came from ... I blame my NAS

@The-EDev
Copy link
Member

don't worry about it, I'm glad you're helping this project forward :)

Copy link
Member

@The-EDev The-EDev left a comment

Choose a reason for hiding this comment

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

Looks good, Will merge once tests are done

@zefrenchy
Copy link
Contributor Author

Success! I feel validated 😅

@The-EDev The-EDev merged commit 610e824 into CrowCpp:master Feb 11, 2022
@The-EDev The-EDev mentioned this pull request Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add sanitizer free loading functions
2 participants