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

Add more support for fs.FS in template parsing #5421

Merged
merged 48 commits into from
Sep 20, 2024

Conversation

doug-threatmate
Copy link
Contributor

@doug-threatmate doug-threatmate commented Jul 20, 2024

Proposed changes

This adds a new LoadHelperFileFunction option to Options, which allows the user to specify a function to use to load a helper file (since the current one does not respect the catalog yet). This can be used to fully implement template parsing using a custom fs.FS without ever needing to touch the file system.

This adds more support for fs.FS in the disk catalog for completeness. This fixes some places where direct os file-related calls were being made to use the catalog interface instead.

Note that the JavaScript compiler still does not work in any context where the pkg/js/libs/fs package is used. In particular, the ReadFilesFromDir function is hard-coded to use the os package and not respect the catalog.

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

ehsandeep and others added 30 commits October 20, 2023 11:57
…ctdiscovery#4252)

Bumps [github.com/gin-gonic/gin](https://github.com/gin-gonic/gin) from 1.9.0 to 1.9.1.
- [Release notes](https://github.com/gin-gonic/gin/releases)
- [Changelog](https://github.com/gin-gonic/gin/blob/master/CHANGELOG.md)
- [Commits](gin-gonic/gin@v1.9.0...v1.9.1)

---
updated-dependencies:
- dependency-name: github.com/gin-gonic/gin
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/docker/docker](https://github.com/docker/docker) from 24.0.5+incompatible to 24.0.7+incompatible.
- [Release notes](https://github.com/docker/docker/releases)
- [Commits](moby/moby@v24.0.5...v24.0.7)

---
updated-dependencies:
- dependency-name: github.com/docker/docker
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@doug-threatmate doug-threatmate marked this pull request as draft July 27, 2024 13:08
@doug-threatmate doug-threatmate marked this pull request as ready for review July 27, 2024 14:12
@doug-threatmate doug-threatmate changed the title Add more support for fs.FS in the disk catalog Add more support for fs.FS in template parsing Jul 27, 2024
@doug-threatmate
Copy link
Contributor Author

@tarunKoyalwar okay, I think that I've done what you wanted. This implementation is sufficient for my needs. Long-term, I'd love to see more true catalog support, but I think that that will mean fully deprecating a lot of weird local path things that seem to have been carried forward from the old days.

@doug-threatmate
Copy link
Contributor Author

For usage, here's what I'm doing (and it works great):

	ne.Options().LoadHelperFileFunction = func(helperFile, templatePath string, catalog catalog.Catalog) (io.ReadCloser, error) {
		return catalog.OpenFile(helperFile)
	}

Hopefully when we refactor all of the old file system code over the next year or so, we can just use the catalog the way that it was designed to be used (like this) 😂

@doug-threatmate
Copy link
Contributor Author

@tarunKoyalwar do you need anything else from me for this PR?

@doug-threatmate
Copy link
Contributor Author

@tarunKoyalwar anything else you need from me?

@Mzack9999 Mzack9999 self-requested a review September 16, 2024 15:20
Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

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

@doug-threatmate I am unable to push in your branch, so I'd recommend the following changes:

  • Use a generic function signature
type LoadHelperFileFunc func(helperFile, templatePath string, catalog catalog.Catalog) (io.ReadCloser, error)
  • Replace it in options as follows:
type Options struct {
...
// LoadHelperFileFunc is a function that will be used to execute LoadHelperFile.
// If none is provided, then the default implementation will be used.
LoadHelperFileFunc LoadHelperFileFunc
...
}

Then just add the following in the:

func (options *Options) LoadHelperFile(helperFile, templatePath string, catalog catalog.Catalog) (io.ReadCloser, error) {
if options.LoadHelperFileFunc != nil {
		return options.LoadHelperFileFunc(helperFile, templatePath, catalog)
	}
// same code as before

@doug-threatmate
Copy link
Contributor Author

Updated per @Mzack9999's request

@ehsandeep ehsandeep merged commit 694835c into projectdiscovery:dev Sep 20, 2024
12 checks passed
@ehsandeep ehsandeep removed the request for review from dogancanbakir September 20, 2024 21:11
@doug-threatmate doug-threatmate deleted the more-fs-fixes branch October 9, 2024 21:07
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.

4 participants