-
-
Notifications
You must be signed in to change notification settings - Fork 366
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 support for AdonisJS v6 #838
base: master
Are you sure you want to change the base?
Conversation
packages/adonis/package.json
Outdated
"@bull-board/api": "6.2.1", | ||
"@bull-board/ui": "6.2.1", | ||
"ejs": "^3.1.10", | ||
"mime": "^4.0.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this dep, after the change of @adonisjs/static
}); | ||
|
||
if (this.statics) { | ||
const statics = this.statics; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Manual handling of static files is not the proper way implement it.
Your current implementation opens a SECURITY issue.
Can we use the default @adonisjs/static
addon somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have looked into the @adonisjs/static
and docs here: https://docs.adonisjs.com/guides/basics/static-file-server#static-files-server, but they haven't shared any way to use the static file server beyond the config. It seems to only load static build assets from the public
folder, so I found an example of fs
in packages > h3 > H3Adapter.ts
and implemented that instead.
I also needed to use the mime
package to find the right Content-Type
header to return to render the file correctly.
I will continue to explore Adonis's codebase to see if I can find a way to use the static file server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
h3 package has several checks inorder to prevent path traversal Issue.
Maybe you can implement somthing similar.
} | ||
|
||
public setErrorHandler(handler: (error: Error) => ControllerHandlerReturnType) { | ||
this.errorHandler = handler; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see that there is any usage of this errorHandler from Adonis part... did you missed it?
Hi @atharahmed thank you for this awasom PR 🙏🏽 |
This PR adds an adapter for AdonisJS v6 and tries to solve #797