[myserver-commit] [2752] Code refactoring.

[Giuseppe Scrivano]

Revision: 2752
          http://svn.sv.gnu.org/viewvc/?view=rev&root=myserver&revision=2752
Author:   gscrivano
Date:     2008-08-15 20:37:19 +0000 (Fri, 15 Aug 2008)

Log Message:
-----------
Code refactoring.  Security class methods now require a valid XmlParser 
instance trought the SecurityCache proxy class

Modified Paths:
--------------
    trunk/myserver/include/security.h
    trunk/myserver/include/security_cache.h
    trunk/myserver/src/security.cpp
    trunk/myserver/src/security_cache.cpp

Modified: trunk/myserver/include/security.h
===================================================================
--- trunk/myserver/include/security.h   2008-08-14 23:48:39 UTC (rev 2751)
+++ trunk/myserver/include/security.h   2008-08-15 20:37:19 UTC (rev 2752)
@@ -102,7 +102,9 @@
   SecurityManager();
   ~SecurityManager();
   int getErrorFileName(const char *sysDir, int error, string& out, 
-                       XmlParser* parser=0);
-  int getPermissionMask(SecurityToken* st, XmlParser* parser=0);
+                       XmlParser* parser = 0);
+  int getPermissionMask(SecurityToken* st, XmlParser* parser = 0);
+private:
+  XmlParser* getParser(const char *sysDir);
 };
 #endif

Modified: trunk/myserver/include/security_cache.h
===================================================================
--- trunk/myserver/include/security_cache.h     2008-08-14 23:48:39 UTC (rev 
2751)
+++ trunk/myserver/include/security_cache.h     2008-08-15 20:37:19 UTC (rev 
2752)
@@ -1,6 +1,6 @@
 /*
 MyServer
-Copyright (C) 2005, 2006 Free Software Foundation, Inc.
+Copyright (C) 2005, 2006, 2008 Free Software Foundation, Inc.
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
 the Free Software Foundation; either version 3 of the License, or
@@ -25,22 +25,23 @@
 
 class SecurityCache
 {
-private:
-  /*! Object used to handle security on the server. */
-  SecurityManager sm;
-  /*! Store a list of opened files using a hash dictionary. */
-  HashMap<string, XmlParser*> dictionary;
-  int limit;
 public:
   SecurityCache();
   ~SecurityCache();
   void free();
   void setMaxNodes(int);
   int getMaxNodes();
+  XmlParser* getParser(const char* dir, const char* sys, bool useXpath = true);
        int getSecurityFile(const char* file, const char* sys, string& out);
   int getPermissionMask(SecurityToken* st);
   int getErrorFileName(const char *root, int error, 
                        const char* sysdirectory, string& out);
+private:
+  /*! Object used to handle security on the server. */
+  SecurityManager sm;
+  /*! Store a list of opened files using a hash dictionary. */
+  HashMap<string, XmlParser*> dictionary;
+  int limit;
 };
 
 #endif

Modified: trunk/myserver/src/security.cpp
===================================================================
--- trunk/myserver/src/security.cpp     2008-08-14 23:48:39 UTC (rev 2751)
+++ trunk/myserver/src/security.cpp     2008-08-15 20:37:19 UTC (rev 2752)
@@ -54,8 +54,9 @@
   throttlingRate = (int)-1;
 }
 
+
 /*!
- *Get the error file for a site. 
+ *Get the error file for a page using the specified parser. 
  *Return 0 to use the default one.
  *Return -1 on errors.
  *Return other valus on success.
@@ -66,7 +67,6 @@
                                       XmlParser* parser)
 {
   ostringstream permissionsFile;
-  XmlParser localParser;  
 
   char evalString[64];
   XmlXPathResult* xpathRes;
@@ -74,22 +74,10 @@
 
 
   out.assign("");
-  if(parser == 0)
-  { 
-    permissionsFile << sysDir << "/security" ;
-    if(!FilesUtility::fileExists(permissionsFile.str().c_str()))
-    {
-      return 0;
-    }
-    if(localParser.open(permissionsFile.str().c_str(), true) == -1 )
-      return -1;
-  }
-  else
-  {
-    if(!parser->isXpathEnabled())
-      return 0;
-  }
 
+  if(parser == NULL || !parser->isXpathEnabled())
+    return -1;
+
   sprintf(evalString, "/SECURITY/address@hidden'%d\']/@FILE", error);
   xpathRes = parser->evaluateXpath(evalString);
   nodes = xpathRes->getNodeSet();
@@ -100,8 +88,6 @@
   if(xpathRes)
     delete xpathRes;
 
-  if(parser == 0)
-    localParser.close();
   
   /* Return 1 if both it was found and well configured.  */
   if(nodes && nodes->nodeNr && out.length())
@@ -113,11 +99,10 @@
 
 
 /*!
- *Get the permissions mask for the file[filename]. 
- *The file [directory]/security will be parsed. If a [parser] is specified, 
- *it will be used instead of opening the security file.
- *[providedMask] is the permission mask that the [user] will have 
- *if providing a [requiredPassword].
+ *Get the permissions mask for the file FILENAME using the XML parser PARSER.
+ *The file DIRECTORY/security will be parsed.
+ *PROVIDEDMASK is the permission mask that the [user] will have 
+ *if providing a REQUIREDPASSWORD.
  *Returns -1 on errors.
  */
 int SecurityManager::getPermissionMask(SecurityToken *st, XmlParser* parser)
@@ -148,7 +133,6 @@
   xmlAttr *attr;
   xmlNode *node = 0;
 
-  XmlParser localParser;
   xmlDocPtr doc;
   
   /*
@@ -173,42 +157,16 @@
     return -1;
   if(st->filename == 0)
     return -1;
+  
+  if(parser == NULL)
+    return -1;
 
-  /* 
-   *If there is not specified the parser to use, create a new parser
-   *object and initialize it. 
-   */
-  if(parser == 0)
-  {
-    permissionsFile << st->directory << "/security";
+  doc = parser->getDoc();
 
-    /* If the specified file doesn't exist return 0.  */
-    if(!FilesUtility::fileExists(permissionsFile.str().c_str()))
-    {
-      return 0;
-    }
-    else
-    {
-      if(localParser.open(permissionsFile.str().c_str(), true) == -1)
-      {
-        return -1;
-      }
-      doc = localParser.getDoc();
-    }
-  }
-  else
-  {
-    /* If the parser object is specified get the doc from it.  */
-    doc = parser->getDoc();
-  }
-
-  if(!doc)
-  {
-    if(parser == 0)
-      localParser.close();
+  if(doc == NULL)
     return -1;
-  }
 
+
   /*
    *If the file is not valid, returns 0.
    *Clean the parser object if was created here.
@@ -217,7 +175,6 @@
     node = doc->children->children;
   else if(parser == 0)
   {
-    localParser.close();
     return -1;
   }
 
@@ -534,11 +491,6 @@
     node = node->next;
   }
 
-  if(parser == 0)
-  {
-    localParser.close();
-  }
-
   if(st->providedMask)
   {
     *st->providedMask = 0;

Modified: trunk/myserver/src/security_cache.cpp
===================================================================
--- trunk/myserver/src/security_cache.cpp       2008-08-14 23:48:39 UTC (rev 
2751)
+++ trunk/myserver/src/security_cache.cpp       2008-08-15 20:37:19 UTC (rev 
2752)
@@ -155,44 +155,41 @@
 }
 
 /*!
- *Get the permission mask for the specified file and user. If the security 
file to use
- *is not loaded it will be loaded and added to the cache dictionary for faster 
future 
- *accesses.
+ *Open the XML parser associated to the file.
+ *\param dir The path where start looking.
+ *\param sys The system directory.
+ *\param useXpath Specify if XPath will be used on the file.
  */
-int SecurityCache::getPermissionMask(SecurityToken* st)
+XmlParser* SecurityCache::getParser(const char* dir, const char* sys, bool 
useXpath)
 {
-  XmlParser *parser;
-  string permissionFile;
-  if(st->directory == 0)
-    return -1;
-  if(st->filename == 0)
-    return -1;
+  XmlParser* parser;
+  string file;
 
-  if(getSecurityFile(st->directory, st->sysdirectory, permissionFile))
-    return -1;
+  if(getSecurityFile(dir, sys, file))
+    return NULL;
 
-  parser = dictionary.get(permissionFile.c_str());
+  parser = dictionary.get(file);
 
+
   /*!
-   *If the parser is already present use it.
+   *If the parser is already present and satisfy XPath then use it.
    */
-  if(parser)
+  if(parser && (!useXpath || parser->isXpathEnabled()))
   {
     time_t fileModTime;
     /*! If the file was modified reload it. */
-    fileModTime = FilesUtility::getLastModTime(permissionFile.c_str());
+    fileModTime = FilesUtility::getLastModTime(file.c_str());
     if((fileModTime != static_cast<time_t>(-1))  && 
        (parser->getLastModTime() != fileModTime))
     {
       parser->close();
-      if(parser->open(permissionFile.c_str(), true) == -1)
+      if(parser->open(file.c_str(), useXpath) == -1)
       {
-        dictionary.remove(permissionFile.c_str());
-        return -1;
+        dictionary.remove(file.c_str());
+        return NULL;
       }
 
     }
-    return sm.getPermissionMask(st, parser);
   }
   else
   {
@@ -201,9 +198,9 @@
      */
     XmlParser* old;
     parser = new XmlParser();
-    if(parser == 0)
+    if(parser == NULL)
     {  
-      return -1;
+      return NULL;
     }
 
     if(dictionary.size() >= limit)
@@ -213,87 +210,60 @@
         delete toremove;
     }
 
-    if(parser->open(permissionFile.c_str(), true) == -1)
-      return -1;
+    if(parser->open(file.c_str(), useXpath) == -1)
+      return NULL;
 
-    old = dictionary.put(permissionFile, parser);
+    old = dictionary.put(file, parser);
     if(old)
     {
       delete old;
     }
-    return sm.getPermissionMask(st, parser);  
   }
 
-  return 0;
+  return parser;
 }
 
+
 /*!
- *Get the error file name from the security file.
+ *Get the permission mask for the specified file and user. If the security 
file to use
+ *is not loaded it will be loaded and added to the cache dictionary for faster 
future 
+ *accesses.
  */
-int SecurityCache::getErrorFileName(const char *directory, int error, 
-                                    const char* sysdirectory, string& out)
+int SecurityCache::getPermissionMask(SecurityToken* st)
 {
   XmlParser *parser;
-  string permissionFile;
-  out.assign("");
-  if(directory == 0)
+
+  if(st->directory == 0)
     return -1;
 
-  if(getSecurityFile(directory, sysdirectory, permissionFile))
+  if(st->filename == 0)
     return -1;
 
-  parser = dictionary.get(permissionFile.c_str());
+  parser = getParser(st->directory, st->sysdirectory);
 
-  /*!
-   *If the parser is already present use it.
-   */
-  if(parser)
-  {
-    time_t fileModTime;
-    /*! If the file was modified reload it. */
-    fileModTime = FilesUtility::getLastModTime(permissionFile.c_str());
-    if((fileModTime != static_cast<time_t>(-1))  && 
-       (parser->getLastModTime() != fileModTime))
-    {
-      parser->close();
-      if(parser->open(permissionFile.c_str(), true) == -1)
-      {
-        dictionary.remove(permissionFile.c_str());
-        return -1;
-      }
+  if(parser == NULL)
+    return -1;
 
-    }
-    return sm.getErrorFileName(directory, error, out, parser);
-  }
-  else
-  {
-    /*! 
-     *Create the parser and append at the dictionary.
-     */
-    XmlParser* old;
-    parser = new XmlParser();
-    if(parser == 0)
-    {  
-      return -1;
-    }
+  return sm.getPermissionMask(st, parser);
+}
 
-    if(dictionary.size() >= limit)
-    {
-      XmlParser* toremove = dictionary.remove(dictionary.begin());
-      if(toremove)
-        delete toremove;
-    }
+/*!
+ *Get the error file name from the security file.
+ */
+int SecurityCache::getErrorFileName(const char *directory, int error, 
+                                    const char* sysdirectory, string& out)
+{
+  XmlParser *parser;
 
-    if(parser->open(permissionFile.c_str(), true) == -1)
-      return -1;
+  out.assign("");
 
-    old = dictionary.put(permissionFile, parser);
-    if(old)
-    {
-      delete old;
-    }
-    return sm.getErrorFileName(directory, error, out, parser);  
-  }
+  if(directory == 0)
+    return -1;
 
-  return 0;
+  parser = getParser(directory, sysdirectory);
+
+  if(parser == NULL)
+    return -1;
+ 
+  return sm.getErrorFileName(directory, error, out, parser);
 }