代码之家  ›  专栏  ›  技术社区  ›  Ian Boyd

我应该如何防御?[关闭]

  •  38
  • Ian Boyd  · 技术社区  · 15 年前

    我正在处理一个用于创建数据库连接的小例程:

    以前

    public DbConnection GetConnection(String connectionName)
    {
       ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];
       DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);
       DbConnection conn = factory.CreateConnection();
       conn.ConnectionString = cs.ConnectionString;
       conn.Open();
    
       return conn;
    }
    

    然后我开始查看.NET框架文档,以了解 文件化的 各种事物的行为,看我是否能处理它们。

    例如:

    ConfigurationManager.ConnectionStrings...
    

    这个 documentation 说那个电话 连接字符串 抛出一个 ConfigurationErrorException 如果它无法检索集合。在这种情况下,我无能为力地处理这个异常,所以我将放弃它。


    下一部分是 连接字符串 寻找 连接名 :

    ...ConnectionStrings[connectionName];
    

    在这种情况下, ConnectionStrings documentation 说财产会归还 无效的 如果找不到连接名。我可以检查是否发生了这种情况,并抛出一个异常,让某个人高声说出他们给出的连接名无效:

    ConnectionStringSettings cs= 
          ConfigurationManager.ConnectionStrings[connectionName];
    if (cs == null)
       throw new ArgumentException("Could not find connection string \""+connectionName+"\"");
    

    我重复同样的练习:

    DbProviderFactory factory = 
          DbProviderFactories.GetFactory(cs.ProviderName);
    

    这个 GetFactory 如果指定的工厂 ProviderName 找不到。没有返回的文件 null 但是我仍然可以防守,而且 检查 空值:

    DbProviderFactory factory = 
          DbProviderFactories.GetFactory(cs.ProviderName);
    if (factory == null) 
       throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");
    

    接下来是dbconnection对象的构造:

    DbConnection conn = factory.CreateConnection()
    

    再一次 documentation 没有说明如果它不能创建连接会发生什么,但是我可以再次检查空返回对象:

    DbConnection conn = factory.CreateConnection()
    if (conn == null) 
       throw new Exception.Create("Connection factory did not return a connection object");
    

    接下来是设置连接对象的属性:

    conn.ConnectionString = cs.ConnectionString;
    

    文档没有说明如果无法设置连接字符串会发生什么。是否引发异常?它会忽略它吗?与大多数例外情况一样,如果在尝试设置连接的ConnectionString时发生错误,我将无法从中恢复。所以我什么都不做。


    最后,打开数据库连接:

    conn.Open();
    

    这个 Open method dbconnection是抽象的,所以由dbconnection中下降的提供程序决定它们抛出的异常。抽象的开放方法文档中也没有关于如果出现错误我可以期望发生什么的指导。如果在连接时出错,我知道我无法处理它-我必须让它冒泡,让调用者向用户显示一些UI,然后让他们再试一次。


    public DbConnection GetConnection(String connectionName)
    {
       //Get the connection string info from web.config
       ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];
    
       //documented to return null if it couldn't be found
        if (cs == null)
           throw new ArgumentException("Could not find connection string \""+connectionName+"\"");
    
       //Get the factory for the given provider (e.g. "System.Data.SqlClient")
       DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);
    
       //Undefined behaviour if GetFactory couldn't find a provider.
       //Defensive test for null factory anyway
       if (factory == null)
          throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");
    
       //Have the factory give us the right connection object
       DbConnection conn = factory.CreateConnection();
    
       //Undefined behaviour if CreateConnection failed
       //Defensive test for null connection anyway
       if (conn == null)
          throw new Exception("Could not obtain connection from factory");
    
       //Knowing the connection string, open the connection
       conn.ConnectionString = cs.ConnectionString;
       conn.Open()
    
       return conn;
    }
    

    总结

    所以我的四行函数变成了12行,需要5分钟的文档查找。最后,我捕获了一个允许方法返回空值的情况。但实际上,我所做的只是将一个访问冲突异常(如果我试图调用空引用上的方法)转换为 无效的参数异常 .

    我还发现了两个可能的案例 无效的 返回对象;但我再次只交换了一个例外。

    从积极的方面来说,它的确抓住了两个问题,并解释了异常消息中发生的事情,而不是路上发生的坏事情(即,巴克停在这里)。

    但它值得吗?这是多余的吗?这个防御程序出了问题吗?

    14 回复  |  直到 6 年前
        1
  •  31
  •   JacquesB    15 年前

    手动检查配置并抛出异常并不比让框架在配置丢失时抛出异常更好。您只是复制了框架方法内部发生的前置条件检查,它使您的代码冗长而没有任何好处。(实际上你可能是 去除 通过将所有内容作为基本异常类来传递信息。框架抛出的异常通常更具体。)

    编辑:这个答案似乎有些争议,所以有点精化:防御性编程意味着“为意想不到的事情做好准备”(或者“妄想狂”),而这样做的方法之一就是做大量的前提检查。在许多情况下,这是一个好的实践,但是,正如所有实践一样,成本应该与收益进行权衡。

    例如,抛出“无法从工厂获得连接”异常没有任何好处,因为它没有提到 为什么? 无法获取提供程序-如果提供程序为空,则下一行将引发异常。因此,前提检查的成本(在开发时间和代码复杂性方面)是不合理的。

    另一方面,检查以验证是否存在连接字符串配置 可以 这是合理的,因为异常可以帮助告诉开发人员如何解决问题。您将在下一行中得到的空异常不会告诉丢失的连接字符串的名称,因此您的前置条件检查确实提供了一些值。例如,如果您的代码是组件的一部分,那么这个值就相当大,因为组件的用户可能不知道组件需要哪些配置。

    防御编程的另一种解释是,您不仅应该检测错误条件,还应该尝试从可能发生的任何错误或异常中恢复。总的来说,我不认为这是个好主意。

    基本上,您应该只处理可以处理的异常 一些关于。无论如何都无法从中恢复的异常应该向上传递给顶级处理程序。在Web应用程序中,顶级处理程序可能只显示一般性错误页。但在大多数情况下,如果数据库离线或缺少某些关键配置,则实际上没有太多工作要做。

    有些情况下,这种防御性编程是有意义的,如果您接受用户输入,并且该输入可能导致错误。例如,如果用户提供了一个URL作为输入,并且应用程序试图从该URL中获取某些内容,那么检查该URL是否正确是非常重要的,并且处理请求可能导致的任何异常。这允许您向用户提供有价值的反馈。

        2
  •  13
  •   mqp    15 年前

    嗯,这要看你的观众是谁。

    如果你写的是你希望被很多人使用的库代码,他们不会和你讨论如何使用它,那么这并不是多余的。他们会感激你的努力。

    (也就是说,如果您这样做,我建议您定义更好的异常,而不仅仅是System.Exception,以使希望捕获某些异常但不捕获其他异常的人更容易了解这些异常。)

    但是,如果你只是想自己(或者你和你的朋友)使用它,那么显然它是杀伤力过大的,最终可能会使代码的可读性降低,从而伤害到你。

        3
  •  7
  •   OrionMD    6 年前

    我希望我的团队能像这样编码。大多数人甚至没有得到防御编程的要点。最好的方法是将整个方法包装在一个try catch语句中,并让所有异常由通用异常块处理!

    向你致敬伊恩。我能理解你的困境。我也经历过同样的经历。但您所做的可能帮助了一些开发人员几个小时的键盘攻击。

    记住,当您使用.NET Framework API时,您希望从中得到什么?什么看起来很自然?对代码也要这样做。

    我知道这需要时间。但质量是有代价的。

    附言:你真的不必处理所有的错误并抛出一个自定义的异常。请记住,您的方法只能由其他开发人员使用。他们应该能够自己找出常见的框架异常。那不值得麻烦。

        4
  •  6
  •   Robert Harvey    15 年前

    你的“之前”例子有清晰和简洁的区别。

    如果有什么问题,框架最终会抛出异常。如果您不能对异常做任何处理,那么最好让它向上传播到调用堆栈。

    然而,有时在框架内部抛出一个异常,实际上并不能揭示实际问题是什么。如果您的问题是没有有效的连接字符串,但是框架抛出了一个异常,比如“无效使用空值”,那么有时最好捕获该异常并用更有意义的消息重新引发它。

    我经常检查空对象,因为我需要一个实际的对象来操作,而且如果对象是空的,那么抛出的异常将是倾斜的,至少可以这样说。但我只检查空对象,如果我知道会发生这种情况。一些对象工厂不返回空对象;而是抛出一个异常,在这些情况下检查空值将是无用的。

        5
  •  3
  •   Robert Rossney    15 年前

    我不认为我会写任何这种空引用检查逻辑——至少,不是你写的方式。

    从应用程序配置文件获取配置设置的程序在启动时检查所有这些设置。我通常构建一个静态类来包含该类的属性(而不是 ConfigurationManager )应用程序中的其他位置。这有两个原因。

    首先,如果应用程序配置不正确,它将无法工作。我宁愿在程序读取配置文件时知道这一点,也不愿在将来某个时候尝试创建数据库连接。

    其次,检查配置的有效性并不是依赖配置的对象真正关心的问题。如果您已经预先执行了这些检查,那么让您自己在代码中到处插入检查是没有意义的。(当然,这也有例外——例如,长时间运行的应用程序需要能够在程序运行时更改配置,并将这些更改反映在程序的行为中;在这样的程序中,需要转到 配置管理器 无论何时需要设置。)

    我不会在 GetFactory CreateConnection 也可以打电话。您将如何编写测试用例来执行该代码?你不能,因为你不知道如何使这些方法返回空值-你甚至不知道它是 可能的 使这些方法返回空值。所以你替换了一个问题-你的程序可以 NullReferenceException 在你不理解的情况下——和另一个更重要的情况下:在同样神秘的情况下,你的程序将运行你没有测试过的代码。

        6
  •  1
  •   Nicolas Dorier    15 年前

    我的拇指规则是:

    如果 引发的异常与 来电者。

    因此,nullreferenceexception没有相关的消息,我将检查它是否为空,并抛出一个带有更好消息的异常。 配置错误异常是相关的,所以我不理解。

    唯一的例外是,getConnection的“contract”不必在配置文件中检索连接字符串。

    如果是这种情况,getConnection应该有一个带有自定义异常的约定,即无法检索连接,那么您可以将configurationErrorException包装在自定义异常中。

    另一种解决方案是指定getConnection不能抛出(但可以返回空值),然后向类中添加“exceptionhandler”。

        7
  •  1
  •   OrionMD    6 年前

    缺少方法文档。;-)

    每个方法都有一些定义的输入和输出参数以及定义的结果行为。在您的情况下,比如:“在成功的情况下返回有效的开放连接,否则返回空值(或者根据您的喜好抛出一个XXXException)。记住这种行为,现在您可以决定应该如何进行防御。

    • 如果您的方法应该公开失败的原因和原因的详细信息,那么像您那样做,检查并捕获所有内容,并返回适当的信息。

    • 但是,如果您只对开放式数据库连接感兴趣,或者 无效的 (或某些用户定义的异常)失败时,只需将所有内容包装在try/catch中并返回 无效的 (或某些例外情况)出错时,以及对象。

    所以我想说,这取决于方法的行为和预期的输出。

        8
  •  1
  •   OrionMD    6 年前

    通常,应该捕获特定于数据库的异常,并将其作为更一般的东西重新抛出,例如(假设的) DataAccessFailure 例外。在大多数情况下,更高级别的代码不需要知道您正在从数据库读取数据。

    快速捕获这些错误的另一个原因是,它们通常在消息中包含一些数据库详细信息,例如“no such table:accounts_blocked”或“user key invalid:23424”。如果这会传播到最终用户,那么在以下几个方面是不好的:

    1. 令人困惑的。
    2. 潜在的安全漏洞。
    3. 对你公司的形象感到尴尬(想象一个客户用粗俗的语法阅读错误消息)。
        9
  •  0
  •   Joshua    15 年前

    我会像你第一次那样对它进行编码。

    但是,该函数的用户将使用using块来保护连接对象。

    我真的不喜欢像您的其他版本那样翻译异常,因为这样很难找出它为什么会崩溃(数据库崩溃)?没有读取配置文件等的权限?).

        10
  •  0
  •   Daniel Earwicker    15 年前

    只要应用程序具有 AppDomain.UnexpectedException 处理程序,它转储 exception.InnerException 链和所有堆栈跟踪到某种类型的日志文件(甚至更好,捕获一个小型转储),然后调用 Environment.FailFast .

    从这些信息中,可以相当直接地查明出了什么问题,而无需通过额外的错误检查使方法代码复杂化。

    注意,最好处理 AppDomain.UnexpectedException 并打电话 环境.failfast 而不是顶级的 try/catch (Exception x) 因为对于后者,问题的原始原因可能会被更多的异常所掩盖。

    这是因为如果捕获到异常,则 finally 块将执行,并且可能会引发更多的异常,这将隐藏原始异常(或者更糟的是,它们将删除试图恢复某些状态的文件,可能是错误的文件,甚至是重要的文件)。您永远不应该捕获一个异常,该异常指示您不知道如何处理的无效程序状态-即使是在顶级 main 功能 try/catch 块。处理 AppDomain.UnexpectedException 呼唤 环境.failfast 是一壶不同的鱼,因为它停了 最后 阻止运行,如果你试图停止程序并记录一些有用的信息而不造成进一步的损坏,你肯定不想运行 最后 阻碍。

        11
  •  0
  •   dr. evil    15 年前

    别忘了检查内存不足异常…你知道的 可以 发生。

        12
  •  0
  •   Mark Simpson    15 年前

    伊恩的改变对我来说是明智的。

    如果我使用的是一个系统,并且使用不当,我希望获得关于误用的最大信息。例如,如果我在调用方法之前忘记在配置中插入一些值,那么我需要一个InvalidOperationException,其中包含一条详细说明我的错误的消息,而不是KeynotFoundException/NullReferenceException。

    这都是关于上下文IMO的。我在我的时代看到过一些相当难以理解的异常消息,但其他时候来自框架的默认异常是完全正确的。

    一般来说,我认为最好是谨慎行事,尤其是当你写的东西被其他人大量使用,或者通常是在调用图中很难诊断错误的地方。

    我总是试图记住,作为一段代码或系统的开发人员,比起仅仅使用它的人,我在诊断故障方面处于更好的位置。有时,几行检查代码+一条自定义异常消息可以累计节省数小时的调试时间(也可以使您自己的生活更轻松,因为您不会被拉到其他人的机器上调试他们的问题)。

        13
  •  0
  •   Lucero    15 年前

    在我看来,你的“事后”样本并不是防御性的。因为防御性是检查你控制的部分,这就是论点 connectionName (检查是否为空,并引发ArgumentNullException)。

        14
  •  0
  •   Erik van Brakel scottrakes    15 年前

    为什么不在添加了所有防御性编程之后拆分您拥有的方法呢?您有一堆不同的逻辑块,它们支持不同的方法。为什么?因为然后您封装了属于一起的逻辑,并且您的公共方法只是以正确的方式连接这些块。

    类似这样(在so编辑器中编辑,因此不检查语法/编译器。可能无法编译;-)

    private string GetConnectionString(String connectionName)
    {
    
       //Get the connection string info from web.config
       ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];
    
       //documented to return null if it couldn't be found
       if (cs == null)
           throw new ArgumentException("Could not find connection string \""+connectionName+"\"");
       return cs;
    }
    
    private DbProviderFactory GetFactory(String ProviderName)
    {
       //Get the factory for the given provider (e.g. "System.Data.SqlClient")
       DbProviderFactory factory = DbProviderFactories.GetFactory(ProviderName);
    
       //Undefined behaviour if GetFactory couldn't find a provider.
       //Defensive test for null factory anyway
       if (factory == null)
          throw new Exception("Could not obtain factory for provider \""+ProviderName+"\"");
       return factory;
    }
    
    public DbConnection GetConnection(String connectionName)
    {
       //Get the connection string info from web.config
       ConnectionStringSettings cs = GetConnectionString(connectionName);
    
       //Get the factory for the given provider (e.g. "System.Data.SqlClient")
       DbProviderFactory factory = GetFactory(cs.ProviderName);
    
    
       //Have the factory give us the right connection object
       DbConnection conn = factory.CreateConnection();
    
       //Undefined behaviour if CreateConnection failed
       //Defensive test for null connection anyway
       if (conn == null)
          throw new Exception("Could not obtain connection from factory");
    
       //Knowing the connection string, open the connection
       conn.ConnectionString = cs.ConnectionString;
       conn.Open()
    
       return conn;
    }
    

    PS:这不是一个完整的重构,只做了前两个块。