> 未分类 > [Grbl_Esp32 Issue#231] Allow * for CORS for /upload

[Grbl_Esp32 Issue#231] Allow * for CORS for /upload

未分类 bolang 4个月前 (10-14) 37次浏览

Issue #231 | 状态: 已关闭 | 作者: petervanderwalt | 创建时间: 2019-09-24

[edit] removed dumb question hehe [/edit]
Building my own UI, having trouble uploading due to CORS

评论 (16)

#1 – petervanderwalt 于 2019-09-24

Trying to upload some gcode from my own UI:
Seems CORS is enabled on the ESP32:
See https://github.com/me-no-dev/ESPAsyncWebServer#adding-default-headers

!cors

Will allow me to post from a different IP too

#2 – petervanderwalt 于 2019-09-24

Added Access-Control-Allow-Origin: “*” to the Upload function like this:
!added header

(: hoping to see it in the official builds soon (:

#3 – petervanderwalt 于 2019-09-24

PR created (:

#4 – bdring 于 2019-09-25

1. This creates potencial security concerns. Please change it to be optional via #define in config.h.

It should be disabled by default.

2. We have pull request guidelines in the wiki. Please follow those guidelines.

#5 – petervanderwalt 于 2019-09-25

Potentially yes… , but its only on /upload, its just Grbl, not a door lock or something where security really matters (and it doesnt even offer htttps so a little early to mention security without understanding CORS, what it is and why in this case it causes issues)

Compile time options isnt something i can depend on in my software, cant have users forced to compile themselves. If it cant be default (read up on CORS and consider the context. Security is not the case, i can post from nodejs, and python, etc etc as is. Its just Chrome doing what it should, but in this case ita pointless, thus Allow * is the appropriate thing to do) i cant have an upload button in the software.

#6 – petervanderwalt 于 2019-09-25

https://www.google.com/amp/s/auth0.com/blog/amp/cors-tutorial-a-guide-to-cross-origin-resource-sharing/ explains nicely why in legitimate cases thats exactly what CORS is. Resource sharing

Excuse brevity, typing off mobile, landline internet is down

#7 – luc-github 于 2019-09-26

Sorry as you wrote : Potentially yes... , and as you read in AsyncWebServer it is not enabled by default, and finally as you wrote it legitimate cases not majority of users.
This is security matter and Bart is very sensitive on that.
https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS
so enabling it must be done by user, not by default.

Also it is not a ‘must’ as upload can be done already using curl without CORS:
curl -F 'data=@path/to/local/file' http://192.168.1.100/upload
it is true that the command line authentication is not implemented so
curl -F 'data=@path/to/local/file' http://adminlogin:adminpassword@192.168.1.100/upload won’t work currently, I have implemented on ESP3D but not on grbl due to workload priorities but it matter if your app support authentication.
Also some users wrote some upload app and did not need it I far I know

#8 – luc-github 于 2019-09-26

here a screen shot from Slix App:
!image
and the full video

#9 – petervanderwalt 于 2019-09-26

Closed issue because i wont convince you.

But alas. I know i can upload from “anything but a browser”

Great if you have cam > gcode file > locally running gui that can do the upload > esp32grbl

But you forget its 2019 and browserbased apps can do wonderful things.

Browserbased cam > scans network with webrtc and automatically finds esp32grbl > posts gcode to SD for you. User uses whatever host or webui he prefers to run the file

I cannot now go add a shim users have to download and install to do act as middleman between browser and esp32grbl

Think about it but dont bother replying. If you can post inauthenticated to /upload from curl/vb/python/nodejs etc, why ONLY block browsers in your default behavior. Dont reply just to reply, or to prove you are correct. Sleep on it and think about what i sau by that and think about the potential for all kinds of browser based gcode generators that can do upload straight to SD from browser. Modern days, modern devs, modern ways of thinking.

#10 – luc-github 于 2019-09-26

we did not rejected the PR, we requested modifications for safety concern.
Authentication is enabled by default so user need to disabled it using define.

browser apps block upload if cors header is not present. does this not ring a bell to you ?
if it was not important this restriction won’t exist
what the problem to let user to know they need to open widely their door for your application ?. they will have to compile the fw according their hardware, I do no see any issue to mention them they need to enable a define to make upload working with your application in same way they disable authentication.

modern ways of thinking include web security and majority of users concerns.

#11 – petervanderwalt 于 2019-09-26

Fair enough they have to compile for hardware anyway, no prebuilt hex like Grbl. Alright you win. But i wont have time to work on that for weeks if you like the idea, please add it. I dont have time

#12 – Slixxor 于 2019-09-26

>
>
> Fair enough they have to compile for hardware anyway, no prebuilt hex like Grbl. Alright you win. But i wont have time to work on that for weeks if you like the idea, please add it. I dont have time

If you can’t figure this one out, this may help you. It’s the code for our upload in vb.NET.

It uses a WebClient object (which is basically a web browser shell)

Private Sub UploadFile(ByVal FileName As String, ByVal IPAddress As String) Dim IPAdd As String() = Split(IPAddress, ":") lblUploadPcent.Visible = True
SDFailed = False
cmdUploadJob.Enabled = False
CurrState = ChipState.UploadingFile Dim uriString As New System.Uri("http://" & IPAdd(0) & "/upload")
WC.UploadFileAsync(uriString, "POST", FileName) End Sub
Dim WithEvents WC As New System.Net.WebClient()

#13 – petervanderwalt 于 2019-09-26

No got it working, i really just didnt want a nodejs based shim (just browser)

For now I use a proxy on my local nodejs shim (force user to download and install an app, not what i really want, but oh well, will find other uses for it)

browser based gcode generator -> post to local webserver running on shim _> nodejs post to ESP32GRBL: ( Seeing as its so “secure” hehe but I can still post from anywhere except the browser)

“
function esp32grblupload(url, filename, gcode) {
const r = request.post({
url: url,
formData: formData
}, function optionalCallback(err, httpResponse, body) {
if (err) {
return console.error('upload failed:', err);
}
console.log('Upload successful! Server responded with:', body);
});
const form = r.form();
form.append('path', '/');
form.append('myfile', new Buffer(gcode), {
filename: filename
});
}
“

Speaking of security, shouldnt /upload be protected by username/password (or is it – just thought of it now – don’t see auth in there at the moment)

#14 – petervanderwalt 于 2019-09-26

That said, i still hope when the egos and tempers calm down, that they would actually read up about CORS: https://stackoverflow.com/questions/20164173/is-cors-considered-bad-practice (;

I cannot get the point across in words, but hey who knows in future maybe (:

Just to jot it down for others:
– CORS allows sharing of resources (in this case the /upload api on the host, so that people can POST there from other browser based apps (not just apps served from SD – which is limited and slow)
– Considering the use case its totally the correct solution (if you understand the role of the browser, and why it looks for the header before doing a post)
– As-is its not like having CORS disabled is making it any more secure, I can still post to /upload from anywhere else, just not a browser running on a different domain
– fair enough if they want it as a compile time option, but I still dont see the point in this context (unless they also then disable uploads from any other ip/source using a compile time option too. What goes for one should go for all)

#15 – bdring 于 2019-09-26

I think egos and tempers will calm down. We don’t know each other very well yet, but we are all working towards the same thing…a robust and feature rich product.

I do most of the machine control and hardware side of the firmware and @luc-github does all of the networking side of things. Luc is currently “on vacation”. This issue moved a little fast, going to a pull request before either of us had a chance to read the issue.

It sounds like you are trying to do some really cool stuff and I would like to support that. We should at least start with a #defined option to allow it, and down the road reconsider the default scenario. I also have some ideas that might need CORS in the future.

Virtually everyone has to configure GrblESP32. The default is a “test drive” mode that does not use any I/O. To make compiling easier, many of the typically altered things in config.h are actually selected in the cpumap. Cpumap is the traditional Grbl name, but in GrblESP32, it has become the place to completely define a machine. Things like homing options are encouraged to be put there. A new user only needs to pick the their machine to get started. Advanced users can tweak other settings if they like.

Therefore, we can setup a new machine for you if you want CORS on for that.

You should consider joining our Slack

#16 – petervanderwalt 于 2019-09-26

Thanks Bdring, appreciate it (;
You do kinda know me, Peter van der Walt, author or Laserweb1-3 and project lead on LW4.
Also work for OpenBuilds (but the ESP32 stuff is purely personal pet projects so don’t let that play into any discussions) where I write everything on software.openbuilds.com (so yeah lots of cool past experience to bring to the table)

Glad to hear you’ll also need CORS (as always, with some wisdom applied, to specific endpoints in addition to auth etc) so will use my shim app for now, till you do (: – no need to push it anymore.

The PR was really well intentioned but oh well (;

@luc-github : Turn off that phone while on holiday! (; I do the same, feel bothered when people talk to me when I should be off – but its my fault for checking email when i should be relaxing. In effect one then feels frustrated without reason sometimes

Will consider slack thanks. In fact right now, because I also want to ask for (or work as PR if you wish) extra ESP commands for some functions only available via HTTP at the moment (like create folder, delete etc)

原始Issue: https://github.com/bdring/Grbl_Esp32/issues/231

喜欢 (0)