Update to Magento 1.9.4 #70
Conversation
|
Same result. Looks fine! # my local test
wget https://github.com/treestonemedia/magento-mirror/archive/magento-1.9.4.zip ;
unzip -q magento-1.9.4.zip ;
diff -r ~/Downloads/magento/ magento-mirror-magento-1.9.4/ ;
touch magento-mirror-magento-1.9.4/difftest.txt ;
echo "difftest" > magento-mirror-magento-1.9.4/difftest.txt ;
diff -r ~/Downloads/magento/ magento-mirror-magento-1.9.4/ ;
rm -rf magento-1.9.4.zip magento-mirror-magento-1.9.4/ |
|
Will download from Magento.com and verify... |
|
Same result;
|
|
Please merge and tag |
|
Same results, verified. Please merge. |
|
@treestonemedia, @spinsch, @JeroenBoersma, @BassemN, @Niko9911: Until this gets merged, feel free to use: https://github.com/SwiftOtter/magento-mirror |
|
@JosephMaxwell Good try, but due many of scripts are tight into this repo, that is no use 😢 |
|
I'll get this merged today. Sorry for the delay!
- David
… On Dec 3, 2018, at 6:08 AM, Niko ***@***.***> wrote:
@JosephMaxwell Good try, but due many of scripts are tight into this repo, that is no use 😢
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Thanks allot |
|
Yes, please tag as |
|
Will do - what does everyone think about prepending the "v" as is standard in some libraries? Will that break composer includes/etc?
v1.9.4.0
- David
… On Dec 3, 2018, at 11:11 AM, David Windell ***@***.***> wrote:
Yes, please tag as 1.9.4.0 instead @drobinson thanks :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I would be hesitant to make a change like that unless there's a clear benefit that we aren't already getting. For me, it would create a little bit of extra work. I have a script that uses this github repo (and the tags inside it) to do a 3-way merge between the new version, client's customized store and the un-modified old version. Changing the way versions are tagged would require updating of any scripts like mine. I wonder if others are in that same boat. Of course, if there's a problem to be solved with that labeling change, and it would benefit the community in other ways, I'd think folks would be receptive. |
|
I agree with @willcodeforfood |
|
I highly disagree to add |
|
@drobinson any chance you can tag this ASAP as 1.9.4.0 as it is holding up our builds at the moment. (p.s. 1.9.3.0 was tagged with the .0) |
|
I'm considering maintaining my own Magento mirror, this update is finally merged after 5 days with the wrong tag... I needed it day 1. No offence but if you don't have the time maybe add some more maintainers so the one first available can take care of the update? |
|
Yes, please add some more maintainers. Our customers expect upgrades to be available from day 1 also. |
|
I am happy to volunteer as a maintainer. I'll be stuck maintaining legacy M1 stores through the June 2020 EoL and have previously made a few contributions to this repo (#69 #66 #61 #54). I usually end up working in my own fork so that I can get the update out more quickly, so it's no extra work to just do that in this repo instead. |
|
👍 for @willcodeforfood |
|
Hi - Tag is updated. RE: more maintainers - Agreed this repo is not getting updated fast enough. I'm not personally using this repository in my day-to-day work. However, I'm apprehensive to add more maintainers given recent events in the JS community and the increased focus from MageCart hacks, etc. Additionally, if you have strong dependencies on this repository in scripts then I'd suggest forking (if you'd like the rich history from this repo) and self maintaining through the 2020 EOL, or scripting download and extraction directly from Magento. Tagging @LeeSaferite for any additional commentary. |
|
I'm not adverse to adding some maintainers as long as the process for merging updates is documented, simple, and followed. I mean, technically we could script this process. Similar to @drobinson, I'm not working on any M1 projects and my time is very finite so any help is always appreciated. I will also echo what @drobinson said about forking this repo. I suggest that you as a service provider fork this repo for your clients. That will allow you to update the repo with the latest version ASAP and then once the mirror is updated and tagged you can use that tag instead to maintain the history. That breaks your client's direct dependance on this repo but allows you to keep the history as well. Anyway, so I have one volunteer, @willcodeforfood, any others? @JosephMaxwell do you want to help maintain this mirror? I know you personally so the trust thing is easy enough. |
|
@LeeSaferite: sure, I can help. We still have a couple of M1 merchants, so we do need these updates made quickly. |
|
We (https://xxxx.fi/) have many Magento 1 shops still in use, see site for references. I would like highly keep in mind previous attacks, like flatmap-stream library. If this repository get's infected it will mean probably over 100 shops to be infected. Total sales on these shops are measured in billions. This is the power what we are dealing with here. I can volunteer to this, if required. Is there possibility to have x approvals for MR before it can be merged to avoid any malicious code injected into this repository? In other hand this would make still new versions available later, but at least would be a compromise while retaining security of this repository. |
| @@ -208,10 +208,10 @@ public function saveAction() | |||
|
|
|||
| //validate attribute_code | |||
| if (isset($data['attribute_code'])) { | |||
| $validatorAttrCode = new Zend_Validate_Regex(array('pattern' => '/^[a-z][a-z_0-9]{1,254}$/')); | |||
| $validatorAttrCode = new Zend_Validate_Regex(array('pattern' => '/[a-z][a-z_0-9]{1,254}[^event]/')); | |||
There was a problem hiding this comment.
This will match any string containing only characters in the set 'evnt' which may not spell "event", such as "ten", "vent", etc.
This does not need to be complicated, it should simply be a separate if clause with a string comparison like:
if ('event' == $data['attribute_code']) {
...
}That would have the added benefit of not breaking existing translations for the original message.
There was a problem hiding this comment.
Will not help much due it is magento core. If your PR mismatches from official magento PR it will be rejected, even if the fixes very good cause
There was a problem hiding this comment.
Yeah, just noting it here for people that use this repo. Also, the regex change broke the validation that the string must start with a character so now other characters can be injected into the attribute code like "foo.bar/abc" will now validate. Whoever wrote that regex obviously doesn't know anything about regex...
There was a problem hiding this comment.
it doesnt matter as the official magento core have same issue. Please create bug report for magento. This is not correct place.
There was a problem hiding this comment.
@colinmollenhour I promise it wasn't us that wrote that regex :)
There was a problem hiding this comment.
This is such an incredibly obvious bug that I think anyone smart enough to exploit it would have easily found it themselves; it took me about two seconds.
I started a discussion here and got your attention so now you can report it if you so feel inclined. 🤣 As for me, until this link is not a 404 for the general public, I don't report Magento bugs: https://github.com/magento/magento1
There was a problem hiding this comment.
As I told you, there is life outside of GitHub. You do not need repository to report it to them. Be smart, not a dummy who needs hand-to-hand tutorial.
Well, that still passed their review. Whatever, I'm done with you if you cannot use your keyboard to report it. Instead of that you're just writing about it in GitHub repositories.
There was a problem hiding this comment.
Here, will make it easy for you to report: https://regex101.com/r/dRFXLm/2/tests
There was a problem hiding this comment.
@Niko9911 can we keep things civil without personal insults please?
There was a problem hiding this comment.
@davidwindell I'm not insulting, but I'm highly questioning why he cannot create bug report as normal people would do instead spamming about it on GitHub.
|
@drobinson I think you need to delete this old tag |
|
@royduin @davidwindell cc/ @drobinson @LeeSaferite sorry for plugging into this regarding composer and version tags, the v in the beginning does not matter much for composer, both is legit. But I would still advice against mixing them and keep with the already used approach. |
No description provided.