Этот код XDocument безопасен? - PullRequest
Новая
       0

Этот код XDocument безопасен?

0 голосов
/ 25 февраля 2011

Я пишу класс с именем Категория , который имеет 2 статических метода для извлечения данных XML из внешнего ресурса.В моем примере ниже я покажу только один, потому что они очень похожи.

Я пытаюсь выяснить, является ли этот код "Безопасным" с точки зрения недействительных URL, недействительных данных и т. Д... в основном сделать его более надежным Вот коды 

private static string XmlUri
    {
        get { return "path-to-xml-file"; }
    }
private static XDocument XmlFile { get; set; }
public int ID { get; set; }
public string Name { get; set; }
public int Parent { get; set; }

/// <summary>
/// Gets a specific category
/// </summary>
/// <param name="id"></param>
/// <returns>A Category with the specified ID</returns>
public static Category Get(int id)
{
    try
    {
        if (XmlFile == null)
            XmlFile = XDocument.Load(XmlUri);
    }
    // Invalid URL or data
    catch (Exception ex)
    {

        // TODO: Log exception
        Console.WriteLine(ex.Message);
    }

    if (XmlFile == null)
        return null;

    var cat = from category in XmlFile.Descendants("Category")
               where category.Attribute("id").Value.ParseSafe() == id
               select new Category
               {
                   ID = category.Attribute("id").Value.ParseSafe(),
                   Parent = category.Attribute("parent").Value.ParseSafe(),
                   Name = category.Value
               };

    return cat.SingleOrDefault();
}

Ответы [ 2 ]

1 голос
/ 25 февраля 2011

Не потокобезопасен, как упоминалось в ChaosPandion.

Непонятное поведение производительности - Get (int) выглядит как простой быстрый метод, но на самом деле включает нетривиальную работу.Используйте ленивую инициализацию и читайте всю коллекцию категорий onece в словарь.

Не следует перехватывать Exception и проглатывать его - либо используйте конкретные (я думаю, IOException и XMLExcepetion в этом случае), либо, по крайней мере, разрешите бросать роковые исключенияобычно.

Очевидно, что если вы не контролируете файл XML, это также может привести к замедлению / зависанию при загрузке безумно больших файлов.В зависимости от поведения читателя и сложности XML (если он предоставлен вам злоумышленником) может вызвать некоторые другие исключения, такие как StackOverflow, убивающий ваш процесс.

1 голос
/ 25 февраля 2011

Определить «безопасно». Ваш код выдаст null, если что-то не так. Я хотел бы рассмотреть (пере) бросок в блоке catch после XDocument.Load (). Чтобы быть в безопасности в не , игнорируя неверный URL.

И это также ставит под сомнение ParseSafe () и SingleOrDefault. Что бы вы хотели случиться, если id отсутствует или имеет дефекты?

И небольшое улучшение: вы можете поместить логику Load-on-Demand в метод получения XmlFile. Облегчает, если вы также хотите, чтобы другие элементы, кроме категории.

...