写点什么

审阅“史上“最烂的代码

  • 2020 年 8 月 14 日
  • 本文字数:3792 字

    阅读完需:约 12 分钟

审阅“史上“最烂的代码

Facebook 上有一个名为“Il Programmatore di Merda”(翻译为“ The Shitty Programmer”,中文含义为“糟糕的程序猿”)的社区, 我经常去浏览。网站经常分享一些糟糕的代码和有关编程的话题。今天,我看到一段令我难以置信的代码:



本周最烂代码


仔细看看,上面的代码错误太多,以至于我不知从何谈起。


如果你是一个初级开发工程师,这篇文章会帮你明白上述代码中存在的一些非常严重的问题,并让你引以为鉴。


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>
复制代码


一时之间,我竟然不知道从何说起。


上述错误大致分为 3 类:


  • 安全问题

  • 基本编程概念问题

  • 代码格式化问题


安全问题

我们非常确定以下代码会在客户端运行,因为它被包装在两个<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; }}
复制代码


我们的代码在某些地方有个叫做apiServices的接口,它公开了一个.sql方法,可以对数据库进行 SQL 操作。


这意味着,如果你在运行上述代码的浏览器上打开控制台,就可以执行各种查询,安全隐患极高。


比如,你无需获得授权就可以这样做:


apiService.sql("show tables;");
复制代码


调用上述 API,代码执行后会返回数据库的所有表名称。


我们暂且假装这不是一个严重的问题。但是请接着往下看:


if (account.username === username &&    account.password === password)
复制代码


所以,作者的意思是直接保存了用户所有的明文密码,而没有对它们进行哈希处理?


这简直不可思议!现在我可以打开 Chrome 浏览器的调试器,直接查看每个用户的明文密码。


我非常确定,很大一部分用户会在社交网络、电子邮件服务、银行账户等服务中使用相同的用户名和密码,想象一下,别人可以在没有任何障碍下就可以拿到你的账户和密码,这得有多可怕。


作者尝试设置登录cookie 的方式也存在问题:


$.cookie('loggedin', 'yes', { expires: 1 });
复制代码


所以按照代码的意思,作者使用 jQuery 设置 cookie,让该 cookie 告知 Web 应用程序用户是否通过身份验证。


好吧,千万不要使用 JavaScript 来设置此类 cookie


如果你有存储此类登陆信息的需求,那么使用 cookie 确实是最常见的解决方案,这没有什么问题!但是使用 JavaScript 设置它们意味着你无法设置httpOnly属性,这会导致每个恶意脚本都能轻而易举地访问和获取你的 cookie 内容。


是的,我知道,他们只是存储'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 类型的参数,最后返回一个布尔类型的值。


所以,下面的代码即使很糟糕(明文密码),但也有一定的意义:


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;}
复制代码


这根本没有任何道理呀。


为什么该函数不去掉always-true条件判断,直接返回 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 用户的 cookie。


我们之前讨论过这样一个问题,我可以在我的 Chrome 控制台输入$ .cookie('loggedin','yes',{expires:1}); 保持认证一整天,甚至都不需要一个帐户。


所以,这个网站到底是怎么确定我是谁的?


也许它只是通过用户名/密码身份验证显示一些私人内容,所以它没有展示任何个人数据。总之,没有人知道代码为什么会这么写。


代码格式化问题

代码格式可能是整个代码中不太重要的部分,但我们可以很容易地判断出该开发人员复制/粘贴了某些网站上的代码。


下面的代码片段,我们可以看到开发者使用了双引号引用字符串:


var username = $("#username").val();var password = $("#password").val();
复制代码


然而,下面的代码却又使用了单引号字符串:


$.cookie('loggedin', 'yes', { expires: 1 });
复制代码


这些看起来可能没有那么重要,但实际上我们可以确定,开发人员可能已经从 StackOverflow 复制粘贴了一些代码,甚至都没有遵循整个代码库的代码规范来重写它们。


当然,这只是一个小问题,但它表明开发人员并不真正关心和理解代码的工作方式,只是希望代码以某种方式工作。


大家不要误会,我每天都会在 Google 上进行搜索,但比起仅仅复制和粘贴代码来实现功能,理解代码的工作原理——比如理解如何设置 Cookie,实际上更为重要。


如果由于某种原因整个进程中断了怎么办?你如何确定是脚本的哪一部分不起作用呢?


总结

我绝对可以确定上面的代码是伪造的。这是我第一次看到使用同步方式进行 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返回查询值(我对此表示怀疑),在内部也必须进行与数据库的连接、执行查询语句并发送返回查询结果,这些过程(你可能已经知道了)明显是不同步的。


但是,即使上面的代码不是伪造的,我也可以确信它是由初级开发人员编写的。


我刚刚开始入行写代码的一段时间里,我很确定自己为之前的公司也写过这么糟糕的代码。


这个锅不能甩给初级开发人员


让我们假设上面的代码是真实的。这里的初级开发人员正在竭尽所能实现功能。


他/她尚未开始学习如何正确处理 SQL 查询、cookie 以及其他需要注意的技术点,这完全可以理解!


高级开发人员应该提供某种形式的指导,以确保初级开发人员可以理解他们的错误,保证这样的错误代码不会在生产环境中使用。


我也可以确认,有些公司其实并不真正在乎开发人员编写的代码质量。


代码能解决问题吗? ——生产环境部署一下就知道了呀。

代码是由初级开发人员编写的,甚至都没有高级开发人员的批准吗? ——部署运行一下就知道结果了呀。


哎,Shit happens!


后记

我在 Reddit对此进行了一番讨论后,一个非常给力的小伙伴分享了下面的 Reddit 话题:


This JavaScript code powers a 1,500 user intranet application


所以,是我错了。这段代码并不是伪造的!


英文原文:


https://www.micheleriva.it/posts/2020-07-31-reviewing-the-worst-piece-of-code-ever


2020 年 8 月 14 日 13:573931
用户头像
王坤祥 日拱一卒,功不唐捐。

发布了 70 篇内容, 共 12.1 次阅读, 收获喜欢 118 次。

关注

评论 3 条评论

发布
用户头像
看到这个apiService.sql()接口, 让我有了一种破坏数据库的冲动 🙄
2020 年 08 月 26 日 15:39
回复
用户头像
我想知道谁给提供的apiService.sql()接口这么刺激 🤣
2020 年 08 月 15 日 00:05
回复
用户头像
So 内网的话可以接受?反正内网怎么快怎么来🤔
2020 年 08 月 14 日 23:17
回复
没有更多了
发现更多内容

百度智能云开物工业互联网平台解决方案亮相2021服贸会成果发布会

百度大脑

人工智能 服贸会

浪潮云洲发布标识解析数据网关产品

浪潮云

工业互联网

阿里官方保姆级Java技术图谱发布!够学到春节了,赶紧收藏!

Java 面试 阿里 大厂 金九银十

"云智一体"全场景智能视频技术与应用解析白皮书下载申请

百度开发者中心

白皮书 云智一体 智能视频

MESI缓存一致性协议

Java 架构 面试 后端

一套基于SpringBoot+MyBatis+Docker实现部署电商系统项目,附项目源码地址及开发文档

程序员知识圈

Java 程序员 架构 面试 编程语言

想要入职阿里P6?最少啃完这本500页Java并发多线程源码笔记

Java 编程 面试 多线程 阿里

开源应用中心|动手自建一个超高度自由的个人知识库,原来这么容易!

开源

☕【Java技术指南】「并发编程专题」Fork/Join框架基本使用和原理探究(基础篇)

浩宇天尚

Java forkjoin forkjoinpool 9月日更

Tapdata Real Time DaaS 技术详解 PART I :实时数据同步

tapdata

App 不想被“点名”,mPaaS 隐私合规检测为开发者护航数字生态建设

蚂蚁集团移动开发平台 mPaaS

移动开发 mPaaS 监管合规 隐私安全

大公司运维监控怎么做?从哪些方面考虑?

行云管家

云计算 运维 运维监控 运维审计 数据监控

被面试官问懵:TCP 四次挥手收到乱序的 FIN 包会如何处理?

华为云开发者社区

TCP 网络 报文 挥手 FIN

阿里后端优化这么恐怖?看完这20W字Java性能实战经验手册,最少P7

Java 阿里巴巴 面试 性能调优 金九银十

一周信创舆情观察(8.30~9.5)

统小信uos

完整版《微服务架构实战》资源分享,掌握微服务架构技术栈相关技能,是从一名普通程序员到资深架构师的必经之路。

程序员知识圈

Java 程序员 架构 面试 编程语言

亚信科技AIDB数据库国产化进程加速,计费上云再下一城

亚信AntDB数据库

实践案例 9月日更 亚信科技AIDB数据库

会员业务基于Cloud KMS的数据安全应用

爱奇艺技术产品团队

数据安全法 Cloud KMS

前端技术概览

数据库 大数据 时序数据库 tsdb 数据智能

C语言中动态内存是如何分配的?

华为云开发者社区

数组 内存 动态内存 内存分配 C语音

首场“说透数字化转型专题讲座”将于 9 月 15 日在天津举办

InfoQ 天津

读了这篇SpringBoot底层原理让我在阿里成功涨薪40%,感谢

Java 编程 面试 涨薪 阿里

数据脱敏是什么意思?有什么好处?

行云管家

数据库 数据安全 数据脱敏 数据库安全

【墨天轮专访第四期】华为云GaussDB苏光牛:发挥生态优势,培养应用型DBA

墨天轮

数据库 华为云 GaussDB

朋友已经拿到了好几个Offer(Java架构师岗),却唯独对心仪大厂“字节跳动”的面试迟迟不敢行动!原因竟出在算法上面。

程序员知识圈

Java 程序员 架构 面试 编程语言

Chrome前端调试技巧分享

华为云数据库小助手

大前端 调试 GaussDB 华为云数据库

交易所刷量机器人定制开发案例(源码搭建)

量化系统19942438797

交易所 做市机器人 自动刷量机器人

让 Serverless 应用开发更简单,Serverless Devs 2.0 全新发布

Serverless Devs

开源 Serverless

iOS 屏幕旋转的实践解析

ZEGO即构

ios 音视频 屏幕旋转

了解JDBC层之QueryDSL

邱学喆

QueryDSL SQLQueryFactory

守护油田安全,EMQ X 在石油石化危化品监测管理中的应用

EMQ映云科技

物联网平台 物联网 IoT 边云协同 emq

审阅“史上“最烂的代码_语言 & 开发_Michele Riva_InfoQ精选文章