🙋🏿 🧥 🍾 Mengurai kode terburuk di dunia 🔤 🏇🏻 👇

Ada satu halaman Facebook Italia. Ini disebut "Il Programmatore di Merda", yang berarti "Programmer yang menyebalkan". Saya suka halaman ini.

Mereka sering menerbitkan kode dan meme yang menjijikkan tentang pemrograman. Tetapi suatu hari saya melihat sesuatu yang sangat menakjubkan di sana.

Potongan kode ini telah mendapatkan gelar kehormatan "karya terbaik" minggu ini.

Saya memutuskan untuk membongkar kode ini, tetapi ada begitu banyak hal yang salah sehingga sulit bagi saya bahkan untuk memilih masalah pertama untuk analisis.

Jika Anda seorang programmer pemula, maka materi saya akan membantu Anda memahami kesalahan besar apa yang dibuat oleh mereka yang menulis kode ini.

28 baris kesalahan kode

Untuk membuatnya lebih nyaman, mari ketikkan kode ini di editor:

<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>

Dan ... Yah, sungguh - saya tidak tahu harus mulai menganalisisnya dari mana. Tetapi Anda masih harus memulai. Saya memutuskan untuk memecah kekurangan kode ini menjadi tiga kategori:

Masalah keamanan.
Masalah dengan pengetahuan pemrograman dasar.
Masalah pemformatan kode.

Masalah keamanan

Kode ini mungkin berjalan di klien. Ini ditunjukkan oleh fakta bahwa itu dibungkus dalam sebuah tag <script>(dan, sebagai tambahan, jQuery digunakan di sini).

Tapi jangan salah paham. Kode ini akan terlihat sama buruknya jika itu untuk server. Tetapi mengeksekusinya pada klien berarti bahwa setiap orang yang dapat membaca kode ini akan memiliki akses ke database yang digunakan di dalamnya.

Mari kita lihat fungsinya 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;
  }
}

Dari analisis kodenya, kita dapat menyimpulkan bahwa di suatu tempat ada objek apiServiceyang memberi kita metode .sqlyang dapat digunakan untuk mengeksekusi kueri SQL ke database. Artinya jika Anda membuka konsol pengembang saat melihat halaman yang berisi kode ini, Anda dapat menjalankan kueri apa pun terhadap database.

Misalnya, Anda dapat menjalankan kueri seperti ini:

apiService.sql("show tables;");

Sebagai tanggapan, daftar lengkap tabel database akan dikembalikan. Dan semua ini dilakukan dengan menggunakan API proyek itu sendiri. Tapi, apa yang sudah ada, mari kita bayangkan bahwa ini bukanlah masalah yang nyata. Mari kita lihat lebih baik ini:

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

Kode ini memberi tahu saya bahwa kata sandi disimpan dalam proyek tanpa hashing. Langkah yang bagus! Ini berarti saya dapat mengambil debugger dan melihat kata sandi pengguna proyek. Saya juga percaya bahwa sebagian besar pengguna menggunakan pasangan nama pengguna / kata sandi yang sama di media sosial, layanan email, aplikasi perbankan, dan tempat serupa lainnya.

Saya juga melihat masalah bagaimana cookie disetel dalam kode ini loggedin:

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

Ternyata itu menggunakan jQuery untuk mengatur cookie yang memberi tahu aplikasi web jika pengguna diautentikasi.

Ingat: jangan pernah menyetel cookie ini menggunakan JavaScript.

Jika Anda perlu menyimpan informasi semacam ini pada klien, maka cookie paling sering digunakan untuk ini. Saya tidak bisa mengatakan hal buruk tentang gagasan seperti itu. Tetapi menyetel cookie menggunakan JavaScript berarti atribut tidak dapat dikonfigurasi httpOnly. Ini, pada gilirannya, berarti bahwa skrip berbahaya apa pun dapat mengakses cookie ini.

Dan ya, saya tahu bahwa hanya sesuatu seperti kunci: pasangan nilai yang disimpan di sini,'loggedin': 'yes', jadi meskipun skrip orang lain membaca sesuatu seperti ini, tidak akan banyak ruginya. Tapi ini adalah praktik yang sangat buruk.

Selain itu, saat saya membuka konsol Chrome, saya selalu dapat memasukkan perintah seperti $.cookie('loggedin', 'yes', { expires: 1000000000000 });. Alhasil, ternyata saya masuk ke situs itu selamanya, bahkan tanpa akun.

Masalah dengan pengetahuan pemrograman dasar

Kami dapat berbicara dan berbicara tentang masalah yang dapat ditemukan dalam potongan kode ini, dan kami tidak punya banyak waktu.

Jadi, jelas sebuah fungsi authenticateUseradalah contoh kode yang ditulis dengan sangat buruk. Ini menunjukkan bahwa orang yang menulisnya kurang memiliki pengetahuan dasar tentang pemrograman.

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;
  }
}

Mungkin alih-alih mendapatkan daftar lengkap pengguna dari database, cukup memilih pengguna dengan nama dan kata sandi tertentu? Apa yang terjadi jika jutaan pengguna disimpan dalam database ini?

Saya sudah membicarakan hal ini, tetapi saya akan mengulangi diri saya sendiri, menanyakan pertanyaan berikut: "Mengapa mereka tidak mencirikan kata sandi di database mereka?"

Sekarang mari kita lihat apa yang dikembalikan fungsinya authenticateUser. Dari apa yang saya lihat, saya dapat menyimpulkan bahwa dibutuhkan dua jenis argumen stringdan mengembalikan satu jenis nilai boolean.

Oleh karena itu, potongan kode berikut ini, meskipun ditulis dengan mengerikan, tidak dapat disebut tidak berarti:

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

Jika Anda menerjemahkannya ke dalam bahasa biasa, Anda mendapatkan sesuatu seperti ini: “Apakah ada pengguna dengan nama X dan dengan kata sandi Y? Jika ya, kembalikan benar. "

Sekarang mari kita lihat bagian kode ini:

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

Omong kosong. Mengapa fungsi tidak kembali saja false? Mengapa dia membutuhkan kondisi yang selalu benar?

Sekarang mari kita analisis kode berikut:

$('#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);
  }
});

Bagian jQuery dari kode ini terlihat bagus. Tetapi masalahnya di sini adalah pengaturan kerja dengan cookie loggedin.

Kami telah mengatakan bahwa, bahkan tanpa akun, Anda cukup membuka konsol Chrome dan menjalankan perintah $.cookie('loggedin', 'yes', { expires: 1 });dan diautentikasi selama satu hari.

Bagaimana halaman ini mengetahui siapa pengguna yang diautentikasi? Mungkin hanya menampilkan sesuatu yang istimewa, diperuntukkan hanya bagi mereka yang berhasil lolos verifikasi username dan password dan tidak menampilkan data pribadi apapun? Kami tidak akan tahu ini.

Masalah pemformatan kode

Styling mungkin merupakan masalah terkecil dan paling tidak signifikan dalam kode ini. Tetapi ini dengan jelas menunjukkan bahwa orang yang membuat kode ini menyalin dan menempelkan masing-masing bagiannya dari suatu tempat.

Berikut adalah contoh penggunaan tanda kutip ganda saat memformat string:

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

Dan di tempat lain, tanda kutip tunggal digunakan:

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

Ini mungkin terdengar tidak penting, tetapi sebenarnya ini memberitahu kita bahwa pengembang mungkin telah menyalin beberapa kode dari, katakanlah, StackOverflow, bahkan tanpa menulis ulang, mengikuti panduan gaya yang digunakan dalam proyek. Tentu saja, ini adalah detail kecil, tetapi ini menunjukkan bahwa pengembang tidak terlalu peduli untuk memahami cara kerja kode. Pengembang ini hanya membutuhkan kode agar berfungsi.

Saya ingin menjelaskan di sini sudut pandang saya tentang masalah ini. Saya menelusuri Google untuk hal-hal yang berhubungan dengan kode setiap hari, tetapi saya pikir jauh lebih penting untuk memahami cara menyetel cookie, misalnya, daripada membuat sepotong kode disalin tanpa berpikir dari suatu tempat yang berfungsi. Bagaimana jika karena alasan tertentu program tersebut berhenti bekerja? Bagaimana seorang programmer yang tidak mengerti apa yang terjadi dalam kodenya menemukan kesalahan?

Hasil

Saya sangat yakin bahwa potongan kode ini palsu. Di sini saya pertama kali melihat contoh eksekusi sinkron dari kueri SQL:

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

Biasanya, tugas-tugas seperti itu diselesaikan sebagai berikut:

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

Mereka juga diselesaikan seperti ini:

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

Bahkan jika metode apiService.sqlmengembalikan hasilnya dalam mode sinkron (yang saya ragu), ia perlu membuka koneksi ke database, menjalankan permintaan, mengirim hasilnya ke titik panggilan. Dan operasi ini (seperti yang Anda duga) tidak dapat dilakukan secara serempak.

Tetapi bahkan jika ini benar-benar kode asli, saya yakin itu ditulis oleh programmer pemula, junior. Saya yakin ketika saya bekerja sebagai programmer untuk minggu-minggu pertama, saya juga menulis kode yang buruk (maaf: D).

Tapi ini bukan kesalahan programmer junior.

Katakanlah ini adalah kode nyata yang berjalan di suatu tempat. Beberapa junior akan berusaha keras agar kode ini berfungsi. Pengembang ini masih harus mempelajari cara menangani kueri SQL, cookie, dan yang lainnya dengan benar. Dan dalam hal ini, itu sepenuhnya normal.

Tetapi kepala programmer ini harus mengontrol pekerjaan, menunjukkan kesalahan, dan membuat pemula memahami kesalahannya. Ini akan mengarah pada fakta bahwa kode yang mengerikan seperti itu tidak berhasil diproduksi.

Saya tahu pasti bahwa ada perusahaan yang tidak peduli dengan kualitas kode yang masuk ke produksi. Apakah kode tersebut menyelesaikan masalah? Jika demikian, itu diterapkan tanpa pertanyaan apa pun. Apakah kode tersebut ditulis oleh seorang junior dan bahkan tidak diuji oleh programmer yang lebih berpengalaman? Dalam produksi itu!

Secara umum, ada kesedihan dalam hidup.

Update per 8 Agustus 2020

Ketika saya menulis artikel ini, saya percaya bahwa kode yang saya analisis adalah palsu. Tetapi setelah membahas materi di Reddit, saya diberi tautan ke utas ini . Ternyata, kode ini digunakan di beberapa sistem internal yang mendukung 1.500 pengguna. Jadi saya jelas salah. Ini adalah kode yang benar-benar nyata.

Pernahkah Anda menemukan fragmen kode yang buruk dalam produksi, penuh dengan segala macam masalah?

Mengurai kode terburuk di dunia