我正在尝试验证HTTP请求,首先我想确保所有信息都已发布(然后我将检查每个传入数据的正确格式),从而确保没有进行形式欺骗攻击。但是,由于此HTTP Post来自一个表单,并且该表单包含将近40个字段,因此我遇到了一个
if
子句,如下所示:机制?有更好的方法吗?#1 楼
您可以将所有变量名放入一个数组中并对其进行循环,如下所示(伪代码):var names = { 'First', 'Second', 'Third' };
for (var name in names) {
if (Request[names[name]] == null) {
throw new ApplicationException('Error');
}
}
// No errors if we reach here
评论
\ $ \ begingroup \ $
好旧的0 1无限规则的应用
\ $ \ endgroup \ $
– jk。
2011年10月13日在7:19
\ $ \ begingroup \ $
Request [names [name]]对我来说看起来很奇怪,难道不是Request [name]吗?
\ $ \ endgroup \ $
– ammoQ
2011年10月13日在9:46
\ $ \ begingroup \ $
@ammoQ:该代码是用伪代码编写的,该伪代码是从C#和JavaScript的混合使用中得出的。循环迭代样式来自JS,但Request [name]更适合C#。对于更奇怪的情况,请注意单引号和数组初始化样式(JS)与null和异常处理(C#)的使用。
\ $ \ endgroup \ $
–蚂蚁
2011年10月13日在9:55
\ $ \ begingroup \ $
@pdr,以C#为例:var requiredFields = new [] {“ First”,“ Second”,“ Third”等...};如果(requiredFields.Any(x => Request [x] == null)){/ *请求不完整* /}
\ $ \ endgroup \ $
–马特·戴维(MattDavey)
2011年10月13日上午10:19
\ $ \ begingroup \ $
@Jan Hudec:在JS中,name是一个整数,表示names数组中的索引。因此,names [name]是正确的。
\ $ \ endgroup \ $
–蚂蚁
2011年10月13日在12:18
#2 楼
看起来不可读。我通常将这样的条件提取到私有布尔函数中,并重新格式化以提高可读性。那将是第一步。
此后可能会进行第二次重构-可能有一个与字段名称相对应的字符串列表,并使用
Request
索引器对其进行迭代,检查null
的值。假设if
的所有部分都在进行相同的无效检查。这基本上就是@Ant在他的答案中提出的内容。要回答您的问题-可以接受多少条件?这是一个主观的东西,但通常是可读的。就个人而言,如果条件的含义不清楚,则提取具有良好有意义名称的重构方法是一个很好的开始。
评论
\ $ \ begingroup \ $
+1将最重要的步骤放在第一位。就个人而言,我永远不会看到在这些情况下添加不必要的迭代器如何使事情更具可读性。
\ $ \ endgroup \ $
– pdr
2011年10月13日上午8:30
#3 楼
我可以将验证提取到自己的方法中:if (RequestIsIncomplete(Request))
{
throw new ApplicationException("Incoming data is incomplete");
}
else
{
// Now trying to validate the format of incoming data
}
private bool RequestIsIncomplete(Request request)
{
if (request["First"] == null)
return false;
if (request["second"] == null)
return false;
// etc...
return true;
}
从沉闷的工作中分离出决策是一个很好的选择。由于分离了繁琐的工作,因此您可以根据需要自由地对其进行重构(老实说,请不要按照我在这里所做的那样进行操作-请参见Ants答案)。
#4 楼
我不知道您使用的是哪种语言(也许是C#?),但是您应该将表单数据中发布的字段称为集合,而不是单独列出它们。我认为您应该在OOP方法中使用包含字段对象的Form对象来执行此操作。这是我的意思的非常原始的示例。实际上,您可能有一个由具体字段类(例如int,文本,文件等)实现的Field接口。 br />
class Field {
public bool isValid = true;
public string errorMessage;
public string value;
public Field(string name, string label, string type, bool required) {
}
}
class Form {
public List<Field> fields;
public bool isValid = false;
public void validate() {
foreach(Field field in fields) {
if(field.required && field.value = "") {
this.isValid = false;
field.isValid = false;
field.errorMessage = field.name+" field is required";
}
}
}
public void loadData(Array request) {
//stuff the request data into the field objects
foreach(KeyValuePair<string, string> kvp in request) {
fields.Where(x => x.name = kvp.Key).first().value = kvp.value;
}
}
}
大多数Web框架都具有Form和Field类,您可以查看一些框架API以获得想法。您还可以准确地声明哪个字段保留为空,而不仅仅是显示通用的
one or more fields are blank
消息。即使这仅用于Web服务,也可以指出40个左右字段中的哪个无效。另一个选择是在集合中定义必填字段并使用循环来检查每个
Request[fieldName]
是否为空。评论
\ $ \ begingroup \ $
是@Keyo。好的OOP方法。但是,这确实使情况复杂化,根本没有任何好处。我更喜欢在UI中进行程序性工作,而在业务,数据访问,服务等方面则是OO。+ 1为新的观点。
\ $ \ endgroup \ $
– Saeed Neamati
2011年10月13日在9:11
\ $ \ begingroup \ $
我不同意没有好处。您可以将所有表单处理代码都放在一个地方。可以使用可以在任何地方使用的新方法和验证规则来重构和扩展它。如果需要,可以为表单对象提供一个渲染功能。我更喜欢在UI中以程序方式工作,而在业务中以OO方式工作...表单验证不是业务逻辑吗?这不是UI代码。我不知道您的上下文是什么,但是在任何具有多种形式的项目中,按程序进行都会变得混乱。
\ $ \ endgroup \ $
– Keyo
2011-10-13 9:22
#5 楼
任何if-statement子句系统都是可以接受的,只要它易于阅读和理解即可。首先,当子句的数量超过一(1)时,请将每个子句放在括号中。这样,您就不必考虑运算符的优先级。
其次,当if行变长时,将其分成多行,每个子句一个。
第三,每当编码线彼此平行时,都应添加空格以使组件对齐。这样一来,您可以立即看到并行性,并确信您没有在其他地方抛出任何多余的字符。
我会将您的示例重新编码为:
if ( ( Request[ "first" ] == null )
|| ( Request[ "second" ] == null )
|| ( Request[ "third" ] == null )
|| (...) )
{
第四,如果子句同时包含AND和OR,请确保在子句中加上括号并更改缩进以显示哪些pices代表其他部分的子组件。而且,当它变得如此复杂时,请考虑将其分解为多个if语句。
评论
\ $ \ begingroup \ $
XLNT !! -我更喜欢逻辑的结尾
\ $ \ endgroup \ $
–戴夫
2011-10-13 22:35
\ $ \ begingroup \ $
我一开始就喜欢它们-易于对齐以提高可读性,并且我可以轻松检查逻辑而不必无视条件。
\ $ \ endgroup \ $
– ANeves认为SE是邪恶的
2011年11月4日在16:41
#6 楼
您可以使用linq使其更简单。var requiredFiels = new [] { "First", "Second", ... };
if (requiredFields.Any(x => Request[x] == null))
throw new ApplicationException(...);
我想我会采用不同的方法。我可能会按照域驱动的方法开发应用程序。这意味着我在系统中已经有代表表单中数据所属的业务实体的类。然后,我只需将表单中发布的数据填充到域对象中,然后将验证数据的责任放在域对象本身中。
但是,这两种技术都不能阻止您进行表单欺骗攻击(您说的是目标)。为了防止这种情况,您应该使用防伪令牌。
#7 楼
通过对所有字段进行简单检查,您已经收到了很好的答案。如果不是这种情况,我建议封装支票。创建一个对象,该对象是
validHTTPrequest
。它包含您所有的四十个字段,并且值设置为您认为正确的值。输入您的输入值,并将其分配给新对象,然后进行相等性检查。如果您已使该对象的equals运算符重载以检查其是否为var并查看它们是否相同,那么您就可以解决问题。您的语句将变为:
if (currentRequest = validRequest)
{
//do stuff
}
else
{
throw new ApplicationException("Incoming data is incomplete");
}
如果有效请求的定义发生变化,则只需修改vaildRequest对象的初始化。
评论
您是否希望它们全部为null,如果这样,那么一个字符串数组const可能会做到,如果不是键/值对应该起作用。您为什么不简单地遍历请求?
@Ramhound:因为其中没有空值。通过遍历您看不到的项目。
这不是代码审查问题。最佳实践不在代码审查范围之内。