Project

General

Profile

Actions

Feature #2451

closed

New Abstraction for Identity Key Certificate

Added by Michael Sweatt about 9 years ago. Updated almost 9 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Security
Target version:
-
Start date:
01/29/2015
Due date:
% Done:

100%

Estimated time:

Description

Refactoring code from the SecPubInfo interface, and more precisely the sec-pub-info-sqlite3 concrete implementation into a series of abstractions.

The general design is as follows:

  • SecIdentityManager - a mechanism class that is used to interact with the TPM, Pib, and Identities
  • SecIdentityInfo - a value-semantic class used to manage information relating to a particular identity, including accessing any keys associated with the Identity
  • SecKeyInfo - a value-semantic type used to manage information relating to a particular key including the type of key and associated certificates
  • DbUtil - a utility class to encapsulate common functionality used to access sqlite database

More complete public interfaces are included below.

A current topic relevant to the design that needs to be settled are the circumstances in which we should throw an exception.

class SecIdentityManager {
// A mechanism class to manage access to identities, TPM, and Pib.
public:
  explicit
  SecIdentityManager(const std::string& dir = "");
  // ACCESSORS
  /**
   * @brief Get TPM Locator
   *
   * @throws SecPublicInfo::Error if the TPM info does not exist
  */
  std::string
  getTpmLocator() const;

  /**
   * @brief Get PIB Locator
  */
  std::string
  getPibLocator() const;

  /**
   * @brief Check if the specified identity already exists
   *
   * @param identityName The identity name
   * @return true if the identity exists, otherwise false
  */
  bool
  doesIdentityExist(const Name& identityName) const;

  /**
   * @brief Get name of the default identity
   *
   * @throws SecPublicInfo::Error if there is no default.
  */
  SecIdentityInfo&
  getDefaultIdentity() const;

  /**
   * @brief Get all the identities from public info, return a vector
   * with the results.
  */
  std::vector<SecIdentityInfo>&&
  getAllIdentities() const;

    /**
   * @brief return the scheme of the PibLocator
   */
  std::string
  getScheme() const;

  // MANIPULATORS
  /**
   * @brief Set the corresponding TPM information to @p tpmLocator
   *
   * If the provided @p tpmLocator is different from the existing one, the PIB will be reset,
   * otherwise nothing will be changed.
   *
   * For legacy issue, the TPM info may not exist (some old PIB content may not have this info),
   * this method will simply set the TPM info as provided without changing anything else. Thus an
   * ideal process of handling old PIB is to check if TPM info exists. If it does not exist,
   * then set it to the default value according to configuration.
  */
  void
  setTpmLocator(const std::string& tpmLocator);

  /**
   * @brief Add a new identity
   *
   * if identity already exist, do not add it again
   *
   * @param identityName The identity name to be added
  */
  void
  addIdentity(const Name& identityName);

  /**
   * @brief Set the default identity
   *
   * @param identityName The default identity name
   */
  void
  setDefaultIdentity(const Name& identity);

    /**
   * @brief Delete an identity and related public keys and certificates
   *
   * @param identity The identity name
  */
  void
  deleteIdentityInfo(const Name& identityName);
};

class SecIdentityInfo {

public: 
    //CREATORS
    /** 
      * @brief Create an a new 'SecIdentityInfo' for the identity with the
        specified 'identityName'.
      *
      * @param identityName The identity name
    */
    SecIdentityInfo(Name& identityName);

    //ACCESSORS 
    bool
    doesPublicKeyExist(const Name& keyName) const;

   /**
    * @brief Generate a key name for this identity
    *
    * @param useKsk If true, generate a KSK name, otherwise a DSK name
    * @return The a "SecKeyInfo" for the new key
    */
    SecKeyInfo&
    getNewKeyInfo(bool useKsk) const;

   /**
    * @brief Get all the keys associated with this identity
    * @return A vector of all keys
    */
    std::vector<SecKeyInfo>&& 
    getAllKeys() const;

   /**
    * @brief Get name of the default key name for this identity
    *
    * @throws SecPublicInfo::Error if there is no default
    *
    * @return The keyInfo for the requested key
    */
    SecKeyInfo&
    getDefaultKey() const;

    /**
    * @brief Get the KeyInfo for the key with the specified 'keyName'.
    *
    * @param keyName The name of the desired key 
    *
    * @return The keyInfo for the requested key
    */
    SecKeyInfo&
    getKey(const Name& keyName) const;

   //MANIPULATORS

   /**
    * @brief Delete a public key and related certificates related to the key
    * of the specifed 'name' associated with this identity.
    *
    * @param keyName The key name
    */ 

    void
    deletePublicKeyInfo(const Name& keyName);

   /**
    * @brief Add a public key to the identity storage.
    *
    * @param keyName The name of the public key to be added
    * @param publicKey Reference to the PublicKey object
    */
    void
    addKey(const Name& keyName, const PublicKey& publicKey);


   /**
    * @brief Set the default key name for the corresponding identity
    *
    * @param keyName The key name
    * @throws SecPublicInfo::Error if the key does not exist
    */
    void
    setDefaultKeyName(const Name& keyName) const;
};

class SecKeyInfo {
public:
  SecKeyInfo(const Name& keyName);

   // ACCESSORS
   /**
    * @brief Check if the specified certificate already exists
    *
    * @param certificateName The name of the certificate
    */
   bool
   doesCertificateExist(const Name& certificateName) const;

   /**
   * @brief Get a shared pointer to identity certificate object 
            from the identity storage corresponding to this Key.
   *
   * @param certificateName The name of the requested certificate
   * @throws SecPublicInfo::Error if the certificate does not exist
   */
   shared_ptr<IdentityCertificate>
   getCertificate(const Name& certificateName) const;

   /**
    * @brief Get all the certificate name in public info
    *
    * @return A vector will all certificates for this key.
    */
   std::vector<IdentityCertificate>&&
   getAllCertificates();    

  /**
   * @brief Get the type of the queried public key
   *
   * @note KeyType is also available from PublicKey instance.
   *       This method is more efficient if only KeyType is needed.
   *
   * @param keyName The name of the requested public key
   * @return the type of the key. If the queried key does not exist, KEY_TYPE_NULL will be returned
   */
  virtual KeyType
  getPublicKeyType(const Name& keyName)

  // MANIPULATOR
  /**
   * @brief Add a certificate to the identity storage.
   *
   * It will add the corresponding public key and identity if they do not exist
   *
   * @param certificate The certificate to be added
   */
  void
  addCertificate(const IdentityCertificate& certificate)

  /**
   * @brief Delete a certificate
   *
   * @param certificateName The certificate name
   */
  void
  deleteCertificateInfo(const Name& certificateName)

  /**
   * @brief Add a certificate into the public key identity storage and set the certificate as the
   *        default one of the default identity
   *
   * @param certificate The certificate to be added
   * @throws SecPublicInfo::Error if the certificate cannot be added (though it is really rare)
   */
  void
  addCertificateAsSystemDefault(const IdentityCertificate& certificate);

};

Files

Pib.jpg (50.8 KB) Pib.jpg Alex Afanasyev, 01/31/2015 04:12 PM
pib.vpp (427 KB) pib.vpp Michael Sweatt, 02/03/2015 05:05 PM
pib.vpp (427 KB) pib.vpp Michael Sweatt, 02/12/2015 06:35 PM
Pib.jpg (64.7 KB) Pib.jpg Michael Sweatt, 02/23/2015 02:33 PM
Pib.jpg (387 KB) Pib.jpg Michael Sweatt, 02/25/2015 10:33 PM

Related issues 3 (0 open3 closed)

Blocks ndn-cxx - Bug #2385: Inconsistent interface / use of interface in PIBRejectedYingdi Yu

Actions
Blocks ndn-cxx - Task #2306: Adjust KeyChain exception handlingClosedYingdi Yu12/16/2014

Actions
Blocks ndn-cxx - Feature #1705: Select DigestSha256 signing method with Identity NameClosedYingdi Yu

Actions
Actions #1

Updated by Alex Afanasyev about 9 years ago

Please use markdown syntax next time. Code blocks should be indented 4 spaces

Actions #2

Updated by Alex Afanasyev about 9 years ago

  • Description updated (diff)
Actions #3

Updated by Alex Afanasyev about 9 years ago

  • Description updated (diff)
Actions #4

Updated by Alex Afanasyev about 9 years ago

The proposed refactoring is a little bit weird and I don't see much value in doing it. We could go further and do more logical separation. I'll upload a picture of what I'm thinking

Actions #5

Updated by Alex Afanasyev about 9 years ago

Actions #6

Updated by Alex Afanasyev about 9 years ago

I can recommend finalizing UML design first. Here is the file I got: https://dl.dropboxusercontent.com/u/45347685/uml/pib.vpp (I'm using VisualParadigm software http://www.visual-paradigm.com/)

Actions #7

Updated by Michael Sweatt about 9 years ago

Did my previous note show up?

Actions #8

Updated by Alex Afanasyev about 9 years ago

the only note from you is note-7

Actions #9

Updated by Michael Sweatt about 9 years ago

I made some updates, but in its core it is quite similar to the version Alex suggested

Actions #10

Updated by Yingdi Yu about 9 years ago

  1. why getPibLocator, getTpmLocator, setTpmLocator are the operations of Identities rather than Pib?

  2. what does the generate method do in Keys?

  3. what does add method actually do in Identities? do we create an identity and then add it? how to create an identity?

  4. we can rename doesExist as has

Actions #11

Updated by Michael Sweatt about 9 years ago

I met this afternoon with Yingdi and we discussed a few updates:

  • The most important decisions reached were that: the constructors for Key and certificate will be private to prevent confusion should a user attempt to "construct" an instance which is not in the backend database.
  • We also determined that some of the functionality relating to Pib and Fibs should be relocated.
  • The 'get' method would not create an new entry in the database if the user looks for an entry that does not exist
Actions #12

Updated by Andrew Brown about 9 years ago

Michael, can you post a JPG of that last diagram? I want to see if Yingdi's questions are answered from #10. I have some other questions (based on Alex's JPG):

  1. Shouldn't there be a setDefault() on Keys to set the default Certificate?
  2. Do begin() and end() actually have something to do with building the object or do they just mean first()/last()?
Actions #13

Updated by Michael Sweatt about 9 years ago

As requested this is an exported image of the diagram.
Note that there is also a Community Edition of Visual Paradigm if commenters want to view the UML graphs directly.

Actions #14

Updated by Michael Sweatt about 9 years ago

And let me respond to your questions:

Q:Shouldn't there be a setDefault() on Keys to set the default Certificate?
A: Fixed, see new chart.

Q: Do begin() and end() actually have something to do with building the object or do they just mean first()/last()?
A: If the question is where you can use the iterators to insert new Identities, no. If the question is where you can use iterators to access an Identity and then "build" it by adding keys and things, the iterator should provide access to the underlying Identity, and I see no reason it could not be modifiable reference (i.e const_iterator, iterator) idiom.

Actions #15

Updated by Andrew Brown about 9 years ago

Thanks; I still don't understand begin() and end()... can you explain what they are there for? What happens when you call one of them?

A couple of other questions:

  • Does Identity extend Name?

  • Can we change 'doesExist' either to 'contains' (the traditional Java collection naming) or 'has' (Yingdi's suggestion)? I think 'contains' describes the method better... personal opinion.

  • I have a hunch that getDefault()/setDefault() will cause less confusion as operations you do on the parent object and not on the collection (e.g. on Key instead of Certificates). If developers have to write ...getKey().getCertificates().getDefault() they will assume that the underlying code retrieves all certificates and then finds the default out of the list; this would be confusing because the most likely implementation is to have the default certificate ID as a foreign key on the Key row and retrieve the default key with one database operation. I would pull those up a level (e.g. getKey().getDefault()).

  • I am assuming Key and Certificate will have methods similar to the ones the currently have (to retrieve DERs, etc.)?

Actions #16

Updated by Michael Sweatt about 9 years ago

Does Identity extend Name?

I do not believe it would. It likely will have a Name but not necessarily be a Name.

Can we change 'doesExist' either to 'contains' ...

Sure contains works for me.

I have a hunch that getDefault()/setDefault() ...

I do not have an issues with this, it seems the logic makes sense either way.

I am assuming Key and Certificate will have methods similar to the ones the currently have (to retrieve DERs, etc.)?

I do not believe so, but let me check with Yingdi.

Actions #17

Updated by Andrew Brown about 9 years ago

Makes sense; so Identity will have a getName() so that we can inspect it.

Re: Key/Certificate-specific methods, you may want to look at https://github.com/named-data/jndn/blob/master/src/net/named_data/jndn/security/certificate/Certificate.java; Jeff has a lot of code in there and I don't know whether it needs to stay or go in the new API. I'll ping him to weigh in.

Actions #18

Updated by Anonymous about 9 years ago

My code for Certificate.java follows the ndn-cxx interface closely: https://github.com/named-data/ndn-cxx/blob/3e8b52e5bd948a58d04d67dc9ed49dc7f9383233/src/security/certificate.hpp .

So I guess Andrew is asking: If you are specifying the new API, should it specify the details of the Certificate API such as addSubjectDescription(), etc.?

Actions #19

Updated by Andrew Brown about 9 years ago

Andrew Brown wrote:

Thanks; I still don't understand begin() and end()... can you explain what they are there for? What happens when you call one of them?

Michael, I was just parading my C++ iterator ignorance; please disregard.

Actions #20

Updated by Michael Sweatt about 9 years ago

I do not think I will be touching the existing Certificate type, if I there is some additional functionality that we want to add to Certificate (I do not believe there is) then I would likely make a new issue for that since it is a bit outside the scope of the discuss for these aggregations.

So I think the get(), etc in the Certificates container would likely just return instances of the existing Certificate type.

Actions #21

Updated by Yingdi Yu about 9 years ago

Just try to catch up the discussion here.

  1. we should follow the naming convention of each language. Take doesExist as an example, for c++, find could be better given the class is designed as a container. Moreover, find provides the functionalities of both doesExist and get, so we do not need to define the later two. For java, it can be named as corresponding one of the standard java containers. (@Michael, it seems that there are still some inconsistency in the diagram, some container has the method find while others have doesExist.)

  2. for the getDefault/setDefault, Andrew made a good suggestion, I just wonder what is the return type, an iterator of the container? Another question is that if we move these methods out of the container, would it be better to give them a more specific name, such as getDefaultIdentity().getDefaultKey().getDefaultCertificate()?

  3. for certificate, we do not create a separate one but just reuse the existing one. However, the existing one also needs certain changes to reflect the new certificate format. For example the addSubjectDescription is no longer needed, as it is removed from the new certificate format.

Actions #22

Updated by Andrew Brown about 9 years ago

Michael, sounds good. Yingdi, all three points sound good.

Actions #23

Updated by Michael Sweatt about 9 years ago

Barring oversights, this is a new design implementing all the suggestions above.
Please look it over and tell me if there are any other concerns.

Actions #24

Updated by Yingdi Yu about 9 years ago

Please address following issues:

  1. The set/getDefaultKey should be member methods of Identity.

  2. The set/getDefaultCertificate should be member methods of Key.

  3. What is Keys:generate()? what is the difference between generate and add?

  4. Why there is no remove in Certificates?

Actions #25

Updated by Junxiao Shi about 9 years ago

I won't review UML because it doesn't contain complete information (eg. parameter types and return types).

Please submit header files to Gerrit, and I can have a look.

Actions #26

Updated by Junxiao Shi about 9 years ago

Comments on commit:edc0bec1c8aab1f6594672303c3577789257e533

The design strictly enforces the hierarchy of identity-key-certificate, which implies the hierarchical trust model, and contradicts with ndn-cxx's goal of supporting multiple trust models.

The plural typenames Identities Keys Certificates are confusing.
They look similar to corresponding singular typenames, which can cause mistakes.
It's better to use more explicit typenames such as IdentityContainer, etc.

Pib:::getDefaultIdentity and Identity::getDefaultKey return mutable references.
If I copy-assign another Identity/Key to the return value of these functions, is the default Identity/Key changed?
If yes, delete setDefaultX method; otherwise, return constant references.

The category of each iterator should be declared (and asserted with a concept check).

Why is ndn::Certificates::Certificate type nested?

Why is ndn::Certificates::iterator dereferenced to Key?

Actions #27

Updated by Michael Sweatt almost 9 years ago

Junxiao Shi wrote:

The design strictly enforces the hierarchy of identity-key-certificate, which implies the hierarchical trust model, and contradicts with ndn-cxx's goal of supporting multiple trust models.

The trust model is created through the namespace of the Identity, which this does not deal with. This merely provides organization for Identities, the Keys for these Identities, and the Certificates for these Keys. A separate concern.

Junxiao Shi wrote:

The category of each iterator should be declared (and asserted with a concept check).

I very well understand that the final design for the iterator will be far more fleshed out, the version shown here is just a stand-in to allow for review of the design of the container. The category of the iterator will depend on the support of the underlying infrastructure. Clearly it is currently a ForwardIterator, which is a lower bound on the functionality required.

Your remaining comments I believe I implemented.

Actions #28

Updated by Michael Sweatt almost 9 years ago

Ok, after speaking to Yingdi, and determining that the iterator will far more likely than not be a ForwardIterator, I went ahead and defined it was such.
I also made an effort to more correctly style the code as well as resolving some issues noticed by Yingdi and more that I myself had missed.

Actions #29

Updated by Yingdi Yu almost 9 years ago

Just upload a new abstraction and implementation, please check it out: http://gerrit.named-data.net/1862. If we agree with the new change, then we will revise the description of this task.

Actions #30

Updated by Junxiao Shi almost 9 years ago

Design questions/problems in commit:eff2b849f1b6b4ab45ee032c824e2c924e1f417a

Pib constructor

Why is Pib constructor protected? How to obtain a Pib instance in a normal program?

Identity constructor

Identity does not have a public constructor; Pib::getIdentity returns Identity and can potential throw.

Suppose a program wants to wrap Pib::getIdentity in a try-catch, and also wants to minimize the scope of try block (which is a good practice).

The following snippet is natural, but won't compile due to the lack of Identity public constructor:

bool
f()
{
  Identity identity;
  try {
    identity = pib.getIdentity(...);
  }
  catch (PibImpl::Error&) {
    return false;
  }
  // work with identity
  return true;
}

And the programmer either has to use a pointer:

bool
f()
{
  Identity* identity = nullptr;
  try {
    identity = &pib.getIdentity(...);
  }
  catch (PibImpl::Error&) {
    return false;
  }
  // work with identity
  return true;
}

or expand the scope of try block:

bool
f()
{
  try {
    Identity identity = pib.getIdentity(...);
    // work with identity
    return true;
  }
  catch (PibImpl::Error&) {
    return false;
  }
}

Same problem occurs with Key constructor and Identity::getKey function.

separate container type

Why not use Pib as a container of Identitys, Identity as a container of Keys, Key as a container of Certificates?

container size

XContainer should have a size_t size() const getter.

Depending on the implementation, it could be more efficient than std::distance(container.begin(), container.end()).

iterator

XContainer::iterator should be declared as an alias of XContainer::const_iterator.

It's okay to have iterator as a constant iterator (rather than a mutable iterator). STL std::set<...>::iterator is a constant iterator since C++11.

Actions #31

Updated by Yingdi Yu almost 9 years ago

Pib constructor

A: Pib instance should be created by KeyChain and stays in KeyChain. User can obtain a reference to the Pib instance through a KeyChain method.

Identity constructor

A: The reason of not exposing the constructor is that we do not want users to be aware of the backend implementation and also do not want users to manipulate backend data set directly through a constructor. But for the problem you mentioned, I think making a public default constructor should be fine. The default constructor will set the backend pointer to nullptr, so it won't be useful until it obtained an actual instance from getIdentity.

separate container type

A: I did this according to the original UML diagram. I am okay with merging them together, but the syntax may look weird

identity.begin() // actually return a iterator to key...

container size

A: We can add a size method.

iterator

A: We can add

typedef const_iterator iterator
Actions #32

Updated by Yingdi Yu almost 9 years ago

  • Status changed from New to Feedback
  • % Done changed from 0 to 80
Actions #33

Updated by Yingdi Yu almost 9 years ago

  • Blocks Bug #2385: Inconsistent interface / use of interface in PIB added
Actions #34

Updated by Yingdi Yu almost 9 years ago

  • Blocks Task #2306: Adjust KeyChain exception handling added
Actions #35

Updated by Yingdi Yu almost 9 years ago

  • Blocks Feature #1705: Select DigestSha256 signing method with Identity Name added
Actions #36

Updated by Yingdi Yu almost 9 years ago

Actions #37

Updated by Yingdi Yu almost 9 years ago

  • Assignee changed from Michael Sweatt to Yingdi Yu
Actions #38

Updated by Junxiao Shi almost 9 years ago

  • Tracker changed from Task to Feature

In reply to note-31:

Pib constructor

Is KeyChain expected to subclass Pib?

Pib constructor is protected.

An alternate is to add KeyChain as a friend. But if we do that, why not making Pib constructor private?

Identity constructor

After adding a default constructor, we also need operator! to test whether the instance is invalid.

Calling any method on an invalid instance should throw an exception (not assertion).

separate container type

Agreed.

Actions #39

Updated by Yingdi Yu almost 9 years ago

Pib constructor

KeyChain will be a friend class of Pib. I am not sure if there any sub-class of Pib, that's why I mark its constructor protected. I can mark it as private for now, and if we really need to have a sub-class (though very unlikely), we can mark it as protected.

Identity constructor

After adding a default constructor, we also need operator! to test whether the instance is invalid.

Calling any method on an invalid instance should throw an exception (not assertion).

How to use the operator!? Are you saying that every user of the Identity instance must check if it is valid before using it? Would that be too cumbersome?

Actions #40

Updated by Junxiao Shi almost 9 years ago

Pib constructor

Add KeyChain as a friend now.

Since Pib could have subclass, it's fine to keep the constructor as protected.

Identity constructor

To ensure semantic correctness and prevent misuse, it's important to throw an exception if some method is called on an invalid instance.

The proper type of exception is std::domain_error.

It's useful to provide Identity::oprator! in case some library function wants to ensure an Identity is valid without triggering or catching exceptions.

To test for valid instance, a caller can write if (!!identity) { ... }.

If the caller is sure that an Identity instance is valid, it doesn't need to use the conditional.

Actions #41

Updated by Yingdi Yu almost 9 years ago

Junxiao Shi wrote:

Pib constructor

Add KeyChain as a friend now.

Since Pib could have subclass, it's fine to keep the constructor as protected.

I do not think friend relationship can be inherited, so should we allow subclass? If so, then we require every subclass to declare KeyChain as a friend. This seems to be a bad idea.

I also realized that a particular Pib instance is explicitly created by KeyChain. Compared to creating a subclass of PibImpl and pass it to the Pib constructor directly, creating a subclass of Pib seems unnecessary. In this way, we only need to change the Pib constructor to accept one more parameter shared_ptr<PibImpl>, and mark KeyChain as a friend of Pib.

Identity constructor

To ensure semantic correctness and prevent misuse, it's important to throw an exception if some method is called on an invalid instance.

The proper type of exception is std::domain_error.

ok

It's useful to provide Identity::oprator! in case some library function wants to ensure an Identity is valid without triggering or catching exceptions.

To test for valid instance, a caller can write if (!!identity) { ... }.

This seems a little bit weird. Is there any problem with operator bool?

Actions #42

Updated by Junxiao Shi almost 9 years ago

Pib constructor

friend relation is not inheritable. If KeyChain decides to subclass Pib, MyPib could have a public constructor.

Otherwise, I agree with having Pib constructor private, and accepting PibImpl (smart) pointer as argument.

Identity constructor

Typically both operator! and operator bool (implicit conversion) are provided.

PibImpl::Error

Many methods can throw PibImpl::Error, where PibImpl is an implementation detail. We should declare Pib::Error and use that in docs, and make PibImpl::Error a subclass of Pib::Error if necessary.

Actions #43

Updated by Yingdi Yu almost 9 years ago

Ok, I will make these changes.

Actions #44

Updated by Junxiao Shi almost 9 years ago

Design problem in commit:3b0f4398b0d3d9eccf775382b285bd98757f836f :

Many PibImpl functions, such as .addIdentity, has noexcept specifier and also returns void.

This may lead difficulty in implementing a PibImpl that relies on an external storage such as a file or a MySQL database.

Suppose said external storage has failed (disk is full or MySQL server is down), it's semantically incorrect to return normally from .addIdentity because the identity isn't added, but the function is forbidden from raising an exception.

The only remaining choice is to invoke std::terminate, but this is not always desirable.

Recommendation:

  • Drop noexcept specifier.
  • Semantic errors (such as calling .setDefaultKeyOfIdentity with a nonexistent key) throw Pib::Error (not PibImpl::Error).
  • Underlying errors (such as inaccessible MySQL server) throw PibImpl::Error, which is not a subclass of Pib::Error.
Actions #45

Updated by Yingdi Yu almost 9 years ago

Then basically every method may throw PibImpl::Error, should we state that for each method, or generally state that in the class description? Also, who should catch the error?

Actions #46

Updated by Junxiao Shi almost 9 years ago

  • PibImpl itself does not throw and requires no \throw PibImpl::Error

  • PibImpl::Error should say:

    \brief represents a non-semantic error
    
    A subclass of PibImpl may throw a subclass of this type when
    there's a non-semantic error, such as a storage problem.
    
  • Pib::Error should say: \brief represents a semantic error

  • Every method in Pib/Identity/Key needs \throw PibImpl::Error error in underlying PibImpl if it invokes a PibImpl method

  • Pib/Identity/Key does not catch PibImpl::Error

Actions #47

Updated by Yingdi Yu almost 9 years ago

My question is: who is expected to catch PibImpl::Error?

Actions #48

Updated by Junxiao Shi almost 9 years ago

who is expected to catch PibImpl::Error?

Not the library. Caller of the library, ie the app, MAY catch PibImpl::Error.

Actions #49

Updated by Yingdi Yu almost 9 years ago

  • Status changed from Feedback to Closed
  • % Done changed from 80 to 100
Actions #50

Updated by Alex Afanasyev over 7 years ago

Actions

Also available in: Atom PDF