版本管理 —— Code Review
版本管理 —— Code Review
简介
结合gitlab,简单叙述code review的流程。
what
- code(代码)
审计人员需要熟悉目标系统所使用的开发语言、这个语言的特性、以及这个语言本身存在的漏洞。从安全的角度来看,不同的语言(例如PHP、ASP、Java)本身的一些特性和漏洞往往也会反映在代码漏洞上(虽然代码漏洞大多数时候是程序没有遵循安全编码规范导致的逻辑漏洞) - context(环境背景)
在进行代码审计之前,了解目标应用系统的环境背景十分重要,我们需要了解:- 正在操作什么类型数据,即系统输入、输出问题。很多漏洞和数据流的流动以及安全处理有关,所以,了解目标应用系统的数据流结构很重要
- 发生数据泄漏时对公司的损失有多大
- Audience(目标用户)
了解当前审计的应用系统的目标用户是谁:- 相对可信任的企业内网用户
- 面向公网的普通用户
- 外部系统、外部服务
- Importance(稳定性)
可用性对应用系统来说也是十分重要的,安全审计人员需要了解如果目标系统突然重启、停机会对企业造成多大的危害。我们知道,不管是"云机房建设标准"中对机房的服务器每年允许的"停止服
务时间"作出最大上限的规定,还是"SLA(Service Level Aggrement)"中对用户承诺的服务质量,应用系统保持稳定运行,并不受DDOS攻击的影响都是十分重要的
why
where
Git Pull Request(分支管理)
工作方式
- 1.创建分支
开发者(研发小组人员)在本地仓库中新建一个专门的分支开发功能,分支基于Master进行创建,创建后在这个分支上进行开发对应功能。 - 2.push分支至公开Bitbucket仓库
开发者(Bill、Jason、Frank)Push分支修改到Public Bitbucket仓库中,如下图所示: - 3.开发者发起Pull Request
开发者通过Public Bitbucket发起一个Pull Request。 - 4.成员Code Rewvie
团队的其它成员review code,讨论并修改。此步骤排查业务实现、代码规范、代码解耦、性能、架构等影响代码质量等问题是否存在。
- 5.合并功能
项目维护者(leader、架构等)合并功能到Master Bitbucket中并关闭Pull Request。
完整流程示意图
常见问题记录
编码习惯
- 方法体长
单个Java方法不能超过65535字节;
The code of method xxx() is exceeding the 65535 bytes limit;
单个Java文件常量个数上限是65536
Too many constants, the constant pool for XXX would exceed 65536 entries; - 缺少注释
完善的注释可以利于系统维护; - 硬编码
测试代码和调试语句未清理; - 日志缺失
生产BUG如果没有日志管理会很难排查线上错误;
编码质量
- 重复造轮子
避免重复编写工具类,如SpringUtils,DataUtils,DateUtils,JsonUtils等。
避免重复编写业务代码,如,在注册用户和登陆用户都创建对象并查询用户是否存在,应该封装成一个方法,供注册用户和登陆用户业务调用。 - 公共数据的使用
出现重复调用情况,不是一次调用,多次使用,在与第三方交互过程中,消耗内存较大。例如获取不常变更的信息(公告或新闻等),需要同时转发给多个第三方,但是每个第三方调用我们等接口时,我们都要去new对象,建立连接获取并封装返回,消耗资源内存。 - 参数过多,未封装VO
参数过多情况,要封装对象,否则修改或删除一个参数,会加大代码的可维护性。 - 采用MBG(MyBatis代码生成器)
采用MBG生成的语句进行数据库操作,但是要insert要用insertSelective,update要用updateSelective,如果涉及到关联检索,多表操作,建议在对应的Mapper.xml和Mapper.java中自行添加。 - 命名规范
要望名知意,不要采用中文,类名驼峰,函数方法命名时,单词首字母小写后面用驼峰。 - 代码实现逻辑复杂
了解业务,从业务涉及角度,明确业务最终目的,避开程序员加戏和冗余业务实现的发生。 - 多余代码
例如Hibernate的先查询在修改,我们完全可以通过主键直接修改。 - 异步处理/同步处理
能异步处理的业务就用异步处理,提高系统执行效率。 - 代码重复
封装函数,尽量服用,避免冗余代码出现。 - 逻辑判断最先执行
逻辑判断先执行,避免无用代码执行,消耗内存。 - 前后逻辑设计
明确各层之间的设计意义,例如:Controller做了param验证,Service就不需要再重复验证。 - TODO / FIXME
标记了TODO和FIXME的一定要实际修复,否则无意义。