Анализ на най-лошия сорс код в света

Оригиналът е на Michele Riva

11
3170

Има една интересна италианска уеб страница във Facebook. Тя се нарича Il Programmatore di Merda, което може да се преведе като лайняния програмист. Харесвам тази страница.

Там често публикуват късове от отвратителен сорс код и мемове за програмирането. Но наскоро видях нещо съвсем потресаващо:

Този блок от сорс код заслужи почетното звание ‘най-доброто за изтеклия месец’.

Реших да анализирам този код, но там има толкова много неправилни неща, че дори ми е трудно да избера от кой проблем да започна анализа.

Ако сте начинаещ програмист, то моят материал може да ви помогне да разберете какви ужасни грешки е допуснал кодерът, който е писал това чудо.

28 грешки в сорс кода

Нека, за да ни бъде по-удобно, да наберем този сорс код в текстов редактор:

<script>
function authenticateUser(username, password) {
  var accounts = apiService.sql(
    "SELECT * FROM users"
  );

  for (var i = 0; i < accounts.length; i++) {
    var account = accounts [i];
    if (account.username === username &&
        account.password === password)
    {
      return true;
    }
  }
  if ("true" === "true") {
    return false;
  }
}

$('#login').click(function() {
  var username = $("#username").val();
  var password = $("#password").val();

  var authenticated = authenticateUser(username, password);

  if (authenticated === true) {
    $.cookie('loggedin', 'yes', { expires: 1 });
  } else if (authenticated === false) {
    $("error_message").show(LogInFailed);
  }
});
</script>

И… Не, наистина не знам откъде да започна. Но все пак трябва да се започне отнякъде, реших да разделя недостатъците в този код на три категории:

  • Проблемите с безопасността
  • Проблемите с основните, базови знания за програмирането
  • Проблемите с форматирането на кода
Проблемите с безопасността

Този код със сигурност се изпълнява на страната на клиента – в компютъра на потребителя. Това най-лесно може да се разбере по това, че е затворен в тага <script> (и освен това, тук се използва jQuery).

Не ме разбирайте неправилно. Този код би изглеждал също толкова ужасно, ако бе предназначен за сървър. Но неговото изпълнение на клиентската машина означава, че всички ще имат достъп до базите данни на този нещастен потребител, стига само малко да разгледат кода.

Да се спрем на функцията authenticateUser:

function authenticateUser(username, password) {
  var accounts = apiService.sql(
    "SELECT * FROM users"
  );

  for (var i = 0; i < accounts.length; i++) {
    var account = accounts [i];
    if (account.username === username &&
        account.password === password)
    {
      return true;
    }
  }
  if ("true" === "true") {
    return false;
  }
}

От анализа на нейния код можем да направим извод, че някъде е наличен обекта apiService, който ни дава достъп до метода .sql, с помощта на който могат да се правят SQL заявки към базата данни. А това означава, че ако преглеждате уеб страница с този код, то можете да отворите конзолата на разработчика и да извършите произволни запитвания към базата данни.

Например, може да се направил следната заявка:

apiService.sql("show tables;");

В отговор ще получите пълния списък с таблиците на базата данни. И всичко това става с използването на собствения API на проекта. Какво да говорим, нека да решим, че това не е кой знае колко голям проблем. Нека по-добре да погледнем ето това:

if (account.username === username &&
    account.password === password

Този код ми показва, че паролите в този проект се записват без хеширане. Отличен ход! Това означава, че мога да взема някой дебъгер и да видя паролите в този проект. Сигурен съм, че голям процент от потребителите използва една и съща двойка потребителско име/парола в социалните мрежи, в електронните пощи, в банковите приложения, в торент тракерите и т.н.

Виждам проблем и в това, по какъв начин в този код се използват текстовите бисквитки loggedin:

$.cookie('loggedin', 'yes', { expires: 1 });

Оказва се, че тук се използва jQuery за поставянето на cookie файл, който съобщава на приложението, че потребителят се е логнал.

Запомнете: никога не слагайте подобно текстови бисквитки с използването на JavaScript.

Ако на клиентската машина е необходимо да се съхранява информация от подобен род, то за тази цел най-често се използват именно cookie. Няма нищо лошо в тази идея. Но вмъкването на бисквитки с JavaScript означава, че е невъзможно да бъде настроен атрибутът httpOnly. А това от своя страна означава, че всеки вредоносен скрипт може да получи достъп до файловете с текстовите бисквитки.

И да, знам че тук се записва само нещо от рода на стандартна двойка ключ:значение, 'loggedin': 'yes', и дори някой чужд скрипт да прочете тази информация, няма да има кой знае каква вреда. Но във всички случаи това е една много лоша практика.

Освен това, ако отворя конзолата на Chrome, винаги мога да въведа команда от рода на $.cookie('loggedin', 'yes', { expires: 1000000000000 });. По този начин ще стане така, че завинаги съм влязъл в този сайт без дори да имам акаунт за него.

Проблемите с базовите знания по програмиране

За проблемите и ужасните решения, които можем да открием в този фрагмент от сорс код, може да говорим и говорим, но времето ни не е чак толкова много.

И така, очевидно е, че функцията authenticateUser е един идеален пример за много лошо написан код. Тази функция показва, че този, който я е писал, няма основни базови познания за програмирането.

function authenticateUser(username, password) {
  var accounts = apiService.sql(
    "SELECT * FROM users"
  );

  for (var i = 0; i < accounts.length; i++) {
    var account = accounts [i];
    if (account.username === username &&
        account.password === password)
    {
      return true;
    }
  }
  if ("true" === "true") {
    return false;
  }
}

Може би вместо да се получава целия списък с потребители от базата данни, е достатъчно да се избере потребителят със зададеното име и парола? А какво би станало, ако в тази база данни е записана информацията за милиони потребители?

Вече казах веднъж, но ще се повторя, като задам следния въпрос? ‘Защо тези хора не хешират паролите в собствената база данни?.

Хайде да погледнем какво връща функцията authenticateUser. От това, което виждам, тя приема два аргумента тип string и връща едно единствено значение тип boolean.

Ето защо, следният фрагмент от кода, въпреки че е написан ужасно, не може да се каже, че е безсмислен:

for (var i = 0; i < accounts.length; i++) {
  var account = accounts [i];
  if (account.username === username &&
      account.password === password)
  {
    return true;
  }

Ако перифразираме на обикновен език, ще получим приблизително следното: ‘Има ли потребител с име X и парола Y?. Ако има, то да се върне true‘.

А сега да погледнем ето този фрагмент от кода:

if ("true" === "true") {
  return false;
}

Съвсем безсмислено. Защо тази функция да не връща просто false? За какво й е условие, което винаги е истина?

А сега нека да анализираме следния блок:

$('#login').click(function() {
  var username = $("#username").val();
  var password = $("#password").val();

  var authenticated = authenticateUser(username, password);

  if (authenticated === true) {
    $.cookie('loggedin', 'yes', { expires: 1 });
  } else if (authenticated === false) {
    $("error_message").show(LogInFailed);
  }
});

Фрагментът на този код, в който се осъществява работата със значенията с използването на jQuery, изглежда нормално. Проблемът е в организацията на работата с бисквитките loggedin.

Вече стана дума за това, че дори и да нямам акаунт мога просто да отворя конзолата на Chrome и чрез командата $.cookie('loggedin', 'yes', { expires: 1 });, да се окажа логант в сайта за един ден.

А по какъв начин тази уеб страница може да разбере, кой именно е автентифициран потребител? Може би тя си извежда нещо собствено, предназначено само за тези, които успешно са преминали проверката за име и парола, без да бъдат извеждани някакви лични данни? Това няма как да разберем.

Проблемите с форматирането на кода

Форматирането е навярно най-малкият и незначителен проблем на този код. Но той съвсем ясно показва, че който се е занимавал с този код, отнякъде е копирал неговите отделни фрагменти.

Ето един пример за използването на двойни кавички при форматирането на редовете:

var username = $("#username").val();
var password = $("#password").val();

А в друго място се използват единични кавички:

$.cookie('loggedin', 'yes', { expires: 1 });

Това може и да изглежда, че не е особено важно, но ни показва, че кодерът е копирал отнякъде фрагменти сорс код, най-вероятно от StackOverflow и при това дори не го е преписал – директно Copy и Paste, като по този начин е копирал и стила на оригинала. Това е дреболия, но показва, че програмистът не се грижи особено много, за да разбере как всъщност работи този код. Този кодер иска да постигне само едно – кодът някак си да работи и толкова.

Нека да изкажа своята гледна точка. Лично аз всеки ден търся в Google по нещо, което е свързано с моя сорс код. Но на мен ми изглежда много по-важно да разбера как правилно да работя с текстовите бисквитки, а не просто без мисъл в главата да копирам и поставя късче чужд код. И какво ще стане, ако програмата по някаква причина спре да работи? По какъв начин един програмист, който не разбира какво става в неговия код, ще открие грешката?

Изводи

Сигурен съм, че този фрагмент от сорс код е някакъв фейк. Така например, именно тук видях пример за синхронно изпълнение на SQL заявка:

var accounts = apiService.sql(
  "SELECT * FROM users"
);

Обикновено, тези задачи се решават ето така:

var accounts = apiService.sql("SELECT * FROM users", (err, res) => {
  console.log(err); // some error
  console.log(res); // query result
});

А може да се направи и ето така:

var accounts = await apiService.sql(
  "SELECT * FROM users"
);

Дори и методът apiService.sql да връща резултатите в синхронен режим (в което аз се съмнявам), то ще трябва да се направи обръщение кум базата данни, за да се изпълни заявката/запитване, и резултатът да се подаде в точката на извикване. Не е трудно да се досетим, че тази операции не могат да се извършват синхронно

Но дори и това да е някой реален код, сигурен съм, че е написан от някой съвсем начинаещ кодер, джуниър. Да си призная, когато бях програмист, първите две-три седмици също пишех подобен код (моля да ме извините :D).

Не, това не е грешка на някой младши програмист.

Нека да си представим, че това е съвсем истински код, който работи някъде. Някой младши кодер е положил немалки усилия, за да накара това чудо да заработи. Но този разработчик трябва да научи как правилно се работи с SQL заявките, с cookie файловете и с всичко останало. От тази гледна точна, това може да се счете за нормално.

Но шефът на този кодер трябва да контролира неговата работа, да му покаже недостатъците, да го накара да разбере грешките си и да се поучи от тях. По този начин този твърде лош код няма да попадне в крайната продукция.

Със сигурност зная, че има компании, които не са особено загрижени за качеството на кода, който отива в крайната продукция. Кодът решава ли поставената задача? Ако това е така – без излишни въпроси, той се добавя в крайния продукт. Кодът е написан от младши кодер и не е проверен от опитен програмист? Какво от това? Слагайте го в крайната продукция и да продаваме. Трябва да се спазват сроковете.

Да, в живота има известна доза огорчения.

Допълнение от 8 август 2020 година

Когато пишех този материал, предполагах, че кодът, който анализирах е фейк. Но след като обсъдихме нещата в Reddit, на мен ми изпратиха ето този линк. Оказа се, че въпросният код се използва в някаква компютърна система с 1500 потребители. Явно не съм бил прав. Това си е съвсем реален код, който се използва ежедневно.

А вие срещали ли сте фрагменти с наистина и откровено лош код, пълен с всякакви проблеми?

3.2 9 гласа
Оценете статията
Абонирай се
Извести ме за
guest
11 Коментара
стари
нови оценка
Отзиви
Всички коментари