Всего 125 687 комментариев

Евгений Шеронов
Евгений Шеронов
23 апреля 2021, 13:05
+2
Вроде к каждой статье пишут про безопасность, не?)

$sql = 'SELECT COUNT(id) FROM modxwb_ms2_order_products WHERE product_id = '.$_POST['id'];
Ну это прям призыв, чтобы пришли люди и грохнули сайт без каких-либо сложностей))

$modx->getObject('modResource', $_POST['id']);
И даже так (и вроде когда используется массив) (обсуждали уже где-то здесь) что-угодно можно пропихнуть.

Всегда когда работаем с целыми числами всего лишь нужно добавить (int) перед чем-угодно и спать спокойно.

А это открывает доступ к другим типам инъекций. 
<input type="hidden" id="orderid" value="{$.get.msorder}">
Возможно, Fenom что-то там и порежет, но надеяться не стоит.
/
Евгений Шеронов
Евгений Шеронов
23 апреля 2021, 12:54
0
Спасибо!
Теперь по замечаниям.
1) Абсолютный путь в целом там и не нужен, оставлю относительный.
Но ещё более не секьюрно давать кому-то доступ в админку))
Там и без этого никаких проблем получить все системные настройки через Fenom.

2) Поправил, в следующем обновлении уже не должно всплывать.
/
Наумов Алексей
Наумов Алексей
23 апреля 2021, 12:26
0
Да и вообще, если посмотреть код метода getCustomerId, увидим что:

— если email пустой, то сделаем его в формате «Получатель_заказа@мой сайт.ру». Класс… т.е. первый купивший Алексей зарегистируется на сайте и создаст пользователя с такой почтой, а заказ от второго Алексея внезапно попадет к первому, т.к. почта-то одинаковая…
Ладно, если мы на фронте этого не выводим всего… но в админке то выглядит так, как-будто все эти заказы от одного пользователя!
/
Кирилл
Кирилл
23 апреля 2021, 12:24
0
Случилась аналогичная ситуация, можно более подробно как решить
/
Sergey
Sergey
23 апреля 2021, 12:20
0
Спасибо большое ))
/
Николай Савин
Николай Савин
23 апреля 2021, 12:19
+1
Нет, я тоже затупил. Неверно подсказал. У вас данные хранятся не в заказе, а в адресе, это другая таблица. Нужно добавить ссылку. 
$extfld_delivery_shop = $msOrder->Address->get('extfld_delivery_shop');
/
Sergey
Sergey
23 апреля 2021, 12:17
0
Спасибо огромное, чет затупил )))
/
Николай Савин
Николай Савин
23 апреля 2021, 12:15
+1
Да нет же
$extfld_delivery_shop = $msOrder->get('extfld_delivery_shop');
/
Sergey
Sergey
23 апреля 2021, 12:13
0
У меня в msOrder поле для самовывоза
select id=«delivery_shop» name=«extfld_delivery_shop» value="{$form['extfld_delivery_shop']}" class=«form-control»

Нужно вот так?
$properties = $msOrder->get('properties');
$delshop = $properties['extfld_delivery_shop'];

И просто вывод {$delshop} )?
/
Артур Шевченко
Артур Шевченко
23 апреля 2021, 12:11
0
Нет.
/
Артур Шевченко
Артур Шевченко
23 апреля 2021, 10:04
+1
Нет, я не приемлю принцип «сдал и меня нет», я поддерживаю свои проекты. И стараюсь заранее все нюансы у заказчика узнать, чтобы доработок было минимум. Понятно, что потом аппетиты могут у заказчика вырасти, тогда и переделаем. А за то, что опытом делитесь респект!
/
Алексей Шумаев
Алексей Шумаев
23 апреля 2021, 09:58
0
Опыт — сын ошибок трудных (©), делюсь, пока минута есть.
Подзаработать потом, это хорошо; главное, чтобы проект не возвращался на доработку неожиданно, как будет что-то всплывать. А то техдолг накопится и через пару лет работа будет ради работы делаться. Ну или работать по принципу «сдал проект — и меня нет», тоже так себе подход )))
/
Артур Шевченко
Артур Шевченко
23 апреля 2021, 09:51
0
Вечно эти опытные программисты находят изъяны и не дают насладиться маленьким триумфом от собственного роста? Я понял вас, спасибо. Что до повторной отправки, можно предусмотреть: пишем в поле comment ссылку domain.ru?msorder=2 и выводим это поле в таблице заказов. Логгирование ошибок в скрипте отправки есть, надо только возвращать false и наверное уведомлять админа. Про чтение json это прям сильно))) А вот когда вырастет, тогда можно будет переделать и ещё немного заработать ?
/
Алексей Шумаев
Алексей Шумаев
23 апреля 2021, 09:39
0
Ну да, как вариант — конкретная реализация от задачи.

Вангую немного на будущее:
— вдруг проект вырастет?
— вдруг выяснится, что народ закрывает страницу не дожидаясь отправки и не получает билеты, а потом жалуется?
— вдруг сбой отправки и вы об этом не узнаете, а повторной отправки не предусмотрено?
— вдруг надо будет ещё что-то делать вместе с отправкой билета?
Это я к тому, что лучше этот процесс перенести полностью на бэк, где вы его будете контролировать.

Чтобы БД не запрашивать (хотя для небольших проектов это не имеет особого значения, да и вообще — кому нужна БД — узкое место же :-) ) — можно в файлы json писать: есть файл -> читаем, получаем указанный заказ, отправляем, удаляем. Если отправка каких-то писем не удалась — пишем в лог и данный файл не удаляем. Однако, имхо, это немного извращение.
/
Артур Шевченко
Артур Шевченко
23 апреля 2021, 09:24
0
Понимаете какое дело, бо́льшую часть времени крон будет работать в холостую. Получается он должен раз в минуту, пусть даже раз в пять минут запускать скрипт который будет делать запрос в БД, и если есть письмо, отправлять, если нет, то ничего не делать. Но мероприятия маленькие до 500-600 человек, это примерно по 1 письму в час, если продажу начинать за месяц, в лучшем случае. В общем у меня нет каких-то обоснованных аргументов, но сам факт того что скрипт будет срабатывать впустую мне не нравится))) Я лучше пятисекудный таймер добавлю, чтобы ajax точно скрипт запустил типа «Отправляем билеты. Ждите 5...4...3...2...1 Билеты отправлены. До скорой встречи!»)))
/
Алексей Шумаев
Алексей Шумаев
23 апреля 2021, 09:13
0
Да, именно на крон. Сохраняем очередь отправки в отдельную таблицу, потом кроном раз в ~ 1 мин отправляем накопившиеся и удаляем из таблицы. Лимит на отправку за один запуск скрипта ~ 5-10 писем, чтобы не иметь проблем с хостером (чтобы за спам 100% не принимали).
/
Артур Шевченко
Артур Шевченко
23 апреля 2021, 09:09
0
Спасибо за комментарии. По первым двум пунктам спорить не буду, особенно по второму. Что касается третьего, изначально так и было, однако отправка письма с вложением при покупке разом больше одного билета вызывала зависание от 3 секунд, возможно это связано с логикой работы ms2, поэтому я решил данный функционал вынести на отдельную страницу, потому как пользователь на неё в любом случае переходит и там он уже ничего не заметит. Если будет сбоить, повешу задачу в крон. А что до ms2 и его нужности, будь к меня скилл повыше, наверное, я его бы не использовал, но с другой стороны ms2 дал почти весь функционал, я ведь совсем немного логику подправил и генерацию билета прикрутил и всё, к тому же ещё оплату подключать, а для ms2 уже есть модули.
/
Алексей Шумаев
Алексей Шумаев
23 апреля 2021, 08:56
+3
Навскидку несколько моментов:
1. «Чтобы избежать повторных отправок, записываем id заказа в куки… » — лучше таки флаг отправки сохранить в БД, например в properties msOrder. Ибо на страницу domain.ru?msorder=1 можно перейти не раз + она запросто может попасть в индекс яндекса (проверено) и будет весело. Запретите индексацию таких страниц в robots.
2. Откликаться будет ТОЛЬКО на ajax запросы: if ($_SERVER['REQUEST_METHOD'] != 'POST')…
Этот код будет работать при любом запросе к данной странице постом.
Меняйте на что-то вроде: 
if(empty($_SERVER['HTTP_X_REQUESTED_WITH']) || $_SERVER['HTTP_X_REQUESTED_WITH'] != 'XMLHttpRequest') die;
3. Отправку писем нужно повесить на событие оформления заказа или иное подходящее.

И я бы такой функционал в принципе не вязал бы с MS2, он тут по факту не нужен )

За публикацию личного опыта — спасибо.
/
Михаил
Михаил
23 апреля 2021, 08:29
0
Надо будет протестировать. Спасибо!
/
Николай Савин
Николай Савин
23 апреля 2021, 08:12
+1
Ровно так же как и родные поля таблицы адреса, через get
$properties = $msOrder->get('properties');
/