-
-
Notifications
You must be signed in to change notification settings - Fork 167
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
allowedOrigins error at server.js line 277 #850
Comments
@djlogan2 thank you for reporting this. As I remember @menelike and @s-ol authored this feature. Guys, can you check this one? Related piece of code; |
Yessir, I implemented the workaround just as I said above. Here is a real world example working in production: export const ImportedPgnFiles = new FilesCollection({
collectionName: "importedPgnFiles",
allowClientCode: false, // Disallow remove files from Client
allowedOrigins: /(.*chessclub.com.*)|(.*localhost.*)/,
allowQueryStringCookies: true,
onBeforeUpload(file) {
// Allow upload files under 10MB, and only in png/jpg/jpeg formats
if (/*file.size <= 10485760 && */ /pgn/i.test(file.extension)) {
return true;
}
return "Please upload PGN file";
},
});
ImportedPgnFiles.allowedOrigins = /(.*chessclub.com.*)|(.*localhost.*)/; |
@djlogan2 great, thank you. I feel like we can close it, unless you wish to update documentation? PRs are welcome P.S. you're checking |
Yes, "pgn" is "Portable Game Notation." I didn't fix the comment. I'll do that now :) Re closing, I suppose it depends upon whether you want your users to have to add another line or use the configuration object. That's not my call. Your doc says it works, and in the code it actually sets allowedOrigin from the configuration object, just incorrectly. In my opinion, if you want users to write an extra line, changing the doc is required, but so is removing the erroneous code. |
Lol, yeah, In looked at the comment, then looked at name of collection I'm open to continue discussing this one, but I need to know why you are the first one to report this. Since it wasn't reported by other developers I assume it's isolated to you. If so, we need to know what's unique about your project, and add this exception to the docs. |
I assumed that I was the only one that needed CORS support. It works fine if your meteor project is deployed to "www.production.com" and your users log on to "www.production.com." What I assume is unique in my case is that I am using multiple domain names. So mup/meteor gets deployed with "www.production.com", but my users logon to "mycustomprefix.production.com". Because meteor was deployed to "www.production.com", the file uploader code was built with a hard coded url for "www.production.com", which incurs a CORS issue. Thus, I had to put in an allowedOrigin of "*.production.com". Then of course, as I stated initially, the file uploader code on the indicated line incorrectly evaluates true/false and discards my setting. |
__Changes:__ - 👨💻 Improve `createIndex` helper - 👨💻 Improve error output when FileSystem destination not writable; Related to #857, thanks to @Leekao - 🐞 Fix custom `allowedOrigins` option for CORS; Closing #850, thanks to @djlogan2; __Notes:__ - 👨🔬 Tested with latest release of `[email protected]`
v2.3.1 __Changes:__ - 👨💻 Improve `createIndex` helper - 👨💻 Improve error output when FileSystem destination not writable; Related to #857, thanks to @Leekao - 🐞 Fix custom `allowedOrigins` option for CORS; Closing #850, thanks to @djlogan2; __Notes:__ - 👨🔬 Tested with latest release of `[email protected]`
@djlogan2 have you had a chance to test |
Hi guys
I believe your code for allowedOrigins has a bug. In server.js, line 277, the if statement does not actually allow for allowedOrigins to contains a regex expression as your documentation states.
The workaround is to add a line after the "new FileCollection" with: obj.allowedOrigins = /xyz/ but obviously this is not ideal, particularly since your documentation states otherwise.
The text was updated successfully, but these errors were encountered: